-
Notifications
You must be signed in to change notification settings - Fork 0
[RFC] (Alternative) Fix FPSCR.FI and rework excp methods #81
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
base: master
Are you sure you want to change the base?
Conversation
Signed-off-by: Víctor Colombo <[email protected]>
Signed-off-by: Víctor Colombo <[email protected]>
Signed-off-by: Víctor Colombo <[email protected]>
Signed-off-by: Víctor Colombo <[email protected]>
if (unlikely(tstat.float_exception_flags & float_flag_invalid)) { \ | ||
float_invalid_op_madd(env, tstat.float_exception_flags, \ | ||
sfprf, GETPC()); \ | ||
} \ |
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.
I'm thinking on the implications of this change. Like is it ok to let the instruction continue (to the next iteration) or an invalid should be treated ASAP? Would this also cause a problem below where *xt = t
is now set before the invalid exception is checked? My tests seemed ok but I didn't test the situation where exceptions are enabled
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.
It may be cheaper if we exit the loop earlier on exceptions, but since this is an unlikely
case, I wouldn't be too worried about it. OTOH, the description of these instructions says that "If a trap-enabled exception occurs in any element of the vector, no results are written to VSR[XT]," so I guess we should call do_float_check_status
before we set *xt
.
target/ppc/fpu_helper.c
Outdated
env->fpscr |= FP_VX; | ||
/* Update the floating-point exception summary */ | ||
env->fpscr |= FP_FX; | ||
if (env->fpscr & FP_VE) { |
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.
This is wrong, the original (in finish_invalid_op_arith
) is fpscr_ve == 0
which is the opposite of what I wrote here. Will fix
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.
This is wrong, the original (in
finish_invalid_op_arith
) isfpscr_ve == 0
which is the opposite of what I wrote here. Will fix
Fixed in fdf5092
Alternative implementation for #76 where a flag parameter (instead of a mask) is used to indicate if the instruction should change the FI bit.