Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

compiletime evaluation of int32 values is not correct #9572

Closed
skilchen opened this issue Oct 30, 2018 · 6 comments
Closed

compiletime evaluation of int32 values is not correct #9572

skilchen opened this issue Oct 30, 2018 · 6 comments
Assignees
Labels
Severe VM see also `const` label

Comments

@skilchen
Copy link
Contributor

Example

import algorithm

static:
  echo "compiletime eval"
  var a = 1'i32

  # without repr the following to commands produce: 
  # Error: illegal conversion from 'int32' to 'int'
  # on 32bit systems. 
  echo repr(not(a)) 
  echo repr(a xor -a)
  # on 32bit systems this sort fails. 
  echo sorted([a, a+1], cmp)
  
block nontstatic:
  echo "runtime eval"
  var a = 1'i32
  echo(not(a))
  echo a xor -a
  echo sorted([a, a+1], cmp)

Current Output

using a 64bit compilation, executing nim c --verbosity:0 -r test.nim produces:

compiletime
4294967294
4294967294
@[1, 2]
runtime eval
-2
-2
@[1, 2]

using 32bit compilation the same command produces:

compiletime eval
4294967294
4294967294
stack trace: (most recent call last)
tnot.nim(13)             tnot
...\lib\pure\algorithm.nim(316) sorted
...\lib\pure\algorithm.nim(296) sort
...\lib\pure\algorithm.nim(227) merge
...\lib\pure\algorithm.nim(25) *
...\lib\pure\algorithm.nim(25, 22) Error: unhandled exception: value out of range

Expected Output

on both 64bit and 32bit systems this should produce:

compiletime eval
-2
-2
@[1, 2]
runtime eval
-2
-2
@[1, 2]

Additional Information

The failing sort of some numbers at compiletime on a 32bit system is the same as reported (among others) in issue #9138. It is a consequence of the silent promotion of the result of xor to some larger value than high(int32).

@krux02 krux02 added the VM see also `const` label label Oct 30, 2018
@krux02
Copy link
Contributor

krux02 commented Oct 30, 2018

Here is the not-bug:
https://github.com/nim-lang/Nim/blob/devel/compiler/vm.nim#L873

You want to fix it?

@skilchen
Copy link
Contributor Author

No, thats not something I am able to fix.

And it is likely that some other parts of Nim depend on this current dubious silent promotion of int32s in the VM.

Things like

result = result +% result shl 10
and
result = result +% result shl 15
will fail on 32bit systems, if the VM no longer silently promotes these ints to something where the left-shift doesn't cause an overflow.

@LemonBoy tried a fix in PR #9409, but this attempt failed at a much earlier stage than the lines quoted above.

@krux02
Copy link
Contributor

krux02 commented Oct 31, 2018

@Araq mentioned these lines might be relevant as well.

proc genNarrow(c: PCtx; n: PNode; dest: TDest) =

of opcNarrowS:

I have my doubts. When I am correct, if opcNarrowS would be evaluated here, it would raise an exception.

@skilchen
Copy link
Contributor Author

skilchen commented Oct 31, 2018

My attempt to sort an int32-array at compiletime on a 32bit system does raise an exception originating from genNarrow.
genNarrow would also produce the failures in e.g. hashes.nim if @LemonBoy's attempted fix would not fail much earlier.

Without using repr to echo the results of not and xor on a 32bit system, the stringification of the result also raises an exception (as mentioned in the comments in my example), because the int32 values have been genNarrowU-ed to int64 behind the scenes.

@krux02 krux02 self-assigned this Oct 31, 2018
@krux02
Copy link
Contributor

krux02 commented Feb 26, 2019

@skilchen I am pretty sure I fixed this bug, can you please verify?

@Araq
Copy link
Member

Araq commented May 15, 2019

I can confirm it works now, unfortunately the CIs don't really support 32 bits.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Severe VM see also `const` label
Projects
None yet
Development

No branches or pull requests

3 participants