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

Incorrect results with uint32 types after +> was made to handle large ints #6245

Closed
p6rt opened this issue May 13, 2017 · 9 comments
Closed
Labels

Comments

@p6rt
Copy link

p6rt commented May 13, 2017

Migrated from rt.perl.org#131306 (status was 'resolved')

Searchable as RT131306$

@p6rt
Copy link
Author

p6rt commented May 13, 2017

From @zoffixznet

Before ef29bb9f41aa5d[^1], the +> op special cased large shifts and large numbers. Now that special-casing was removed,
it works fine in the first few runs, but then starts giving incorrect results (setting MVM_SPESH_DISABLE fixes the issue).

I couldn't repro it with *just* the `+>` op, but only in this combination​:

<Zoffix_> c​: ef29bb9f41aa5d sub rotr(uint32 $n, uint32 $b) { $n +> $b +| $n +< (32 - $b) }; say rotr 1652322944, 18; my $x; for ^1000 { $x = rotr 1652322944, 18 }; say $x
<committable6> Zoffix_, ¦ef29bb9​: «27071659120799␤27071659114496»
<Zoffix_> c​: ef29bb9f41aa5d1 sub rotr(uint32 $n, uint32 $b) { $n +> $b +| $n +< (32 - $b) }; say rotr 1652322944, 18; my $x; for ^1000 { $x = rotr 1652322944, 18 }; say $x
<committable6> Zoffix_, ¦ef29bb9f41aa5d
1​: «27071659120799␤27071659120799»

[1] rakudo/rakudo@ef29bb9f41aa5d

@p6rt
Copy link
Author

p6rt commented May 13, 2017

From @zoffixznet

Worth noting Rakudo change wasn't the only change. The actual fix was in MoarVM​:
MoarVM/MoarVM@362277b

@p6rt
Copy link
Author

p6rt commented May 13, 2017

@p6rt
Copy link
Author

p6rt commented May 13, 2017

From @zoffixznet

Seems <+ is actually (or also) affected

@p6rt
Copy link
Author

p6rt commented May 19, 2017

From @zoffixznet

More detective work (for a couple of screens)​: https://irclog.perlgeek.de/moarvm/2017-05-19#i_14608978

@p6rt
Copy link
Author

p6rt commented May 20, 2017

From @zoffixznet

On Sat, 13 May 2017 07​:26​:05 -0700, cpan@​zoffix.com wrote​:

Before ef29bb9f41aa5d[^1], the +> op special cased large shifts and
large numbers. Now that special-casing was removed,
it works fine in the first few runs, but then starts giving incorrect
results (setting MVM_SPESH_DISABLE fixes the issue).

I couldn't repro it with *just* the `+>` op, but only in this
combination​:

<Zoffix_> c​: ef29bb9f41aa5d sub rotr(uint32 $n, uint32 $b) { $n +> $b
+| $n +< (32 - $b) }; say rotr 1652322944, 18; my $x; for ^1000 { $x =
rotr 1652322944, 18 }; say $x
<committable6> Zoffix_, ¦ef29bb9​: «27071659120799␤27071659114496»
<Zoffix_> c​: ef29bb9f41aa5d1 sub rotr(uint32 $n, uint32 $b) { $n +>
$b +| $n +< (32 - $b) }; say rotr 1652322944, 18; my $x; for ^1000 {
$x = rotr 1652322944, 18 }; say $x
<committable6> Zoffix_, ¦ef29bb9f41aa5d
1​:
«27071659120799␤27071659120799»

[1] rakudo/rakudo@ef29bb9f41aa5d

I added a temporary workaround[^1] as well as tests[^2] to cover this bug.

The issue still needs proper addressing as the workaround merely prevents inlining of +< and +> ops

[1] rakudo/rakudo@2f22b70
[2] Raku/roast@c1d62112f3

@p6rt
Copy link
Author

p6rt commented Aug 2, 2017

From @zoffixznet

jnthn │ A while back I think we had to comment out (or similar) some native candidates because of a spesh bug. There's a chance it was what I just fixed in MoarVM/MoarVM@3f33a8419b

@p6rt
Copy link
Author

p6rt commented Aug 2, 2017

From @zoffixznet

Issue appears to have been fixed. Kludge removed in rakudo/rakudo@d9f51da

@p6rt p6rt closed this as completed Aug 2, 2017
@p6rt
Copy link
Author

p6rt commented Aug 2, 2017

@zoffixznet - Status changed from 'new' to 'resolved'

@p6rt p6rt added the spesh label Jan 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

1 participant