-
Notifications
You must be signed in to change notification settings - Fork 122
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
Fix FXTRACT for 0.0 and -0.0 #4093
Conversation
For context the spec mentions that for fscale:
When testing this, through |
96fe7af
to
6fe0e35
Compare
All good here now - instcounci failure coming from #4092 that needs to land before this one. |
pinging here for review! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems reasonable to me, but I'll leave it to Alyssa for the IR change.
ping @alyssarosenzweig |
instead of And+SubNZCV, just use TestNZ to save an instruction |
|
can you combine these two into a _Bfe instruction? (start=52, count=11 or something) |
reexpress as |
0x800f'ffff'ffff'ffff and 0x3ff0'0000'0000'0000 do successfully inline for logical op instructions. You can see it in the InstCountCI output. |
Oops, right. disregard that part! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
as commented
Behaves like NZCVSelect for FPRs.
Fixes fxtract by returning the correct values for 0.0 and -0.0. We moved the split of fxtract into _sig and _exp, to the opcode dispatcher, to ease some comparisons. Also removed the IR node F80XTRACTStack which is not needed anymore.
Thanks for the comments - I think I have addressed them now. |
Fixes fxtract by returning the correct values for 0.0 and -0.0. We moved the split of fxtract into _sig and _exp, to the opcode dispatcher, to ease some comparisons.
Also removed the IR node F80XTRACTStack which is not needed anymore.
Depends on landing #4092 (commit d62dd50 included here but to be merged separately)