Enable Floating Point Exceptions #1619
Conversation
yitzchak
force-pushed
the
fpe
branch
3 times, most recently
from
cdbed81 to
600e795
Compare
src/core/hashTable.cc
Outdated
| @@ -515,7 +515,8 @@ void HashTable_O::sxhash_eql(HashGenerator& hg, T_sp obj) { | |||
| return; | |||
| } | |||
| case gctools::single_float_tag: { | |||
| hg.addValue0(std::abs(::floor(obj.unsafe_single_float()))); | |||
| float value = obj.unsafe_single_float(); | |||
| hg.addValue0((std::fpclassify(value) == FP_ZERO) ? 0u : float_convert<float>::to_bits(value)); | |||
There was a problem hiding this comment.
does float_convert not work on zeroes somehow? If it doesn't, this should probably be mentioned in a comment in float_convert.
There was a problem hiding this comment.
No the issue is that standard requires that -0.0 and 0.0 behave the same in hashes.
There was a problem hiding this comment.
okay, that makes sense for the others, but for an eql hash they should be distinct, right? Since they're not eql? Unless there's a hash table specific rule I'm missing here.
There was a problem hiding this comment.
In addition to the tests in sxhash.lsp it is tested by gethash in all varieties.
There was a problem hiding this comment.
Actually, I'm wrong. The only gethash test that requires that is equalp. Apologies, I'll fix it.
| // printf("Enter handle_fpe Signo: %d Errno:%d Code:%d\n", (info->si_signo), (info->si_errno), (info->si_code)); | ||
| // init_float_traps(); // WHY | ||
| if (_lisp) { | ||
| feenableexcept(_lisp->getTrapFpeBits()); |
There was a problem hiding this comment.
handle_fpe is a signal handler and should be as reentrant as possible. It would be possible for an interrupt to occur between enable and disable here - true elsewhere as well of course. Is there any way we can do this atomically? I imagine not, but it would be nice.
Also, why is this code restoring the floating point traps necessary? Does receiving an FPE warp the traps somehow? An explanation in a comment would be appreciated. Also an explanation for why the if (_lisp) check - because this can happen extremely early, maybe?
There was a problem hiding this comment.
The interrupts should get queued. On amd64 the traps are all reset on SIGFPE so you have to restore them. This is not true on Aarch64.
There was a problem hiding this comment.
The if (_lisp) check is just there because I am paranoid.
There was a problem hiding this comment.
Ok, could you mention the reset in a comment? Also, do you have a doc reference for this or is it just what you've observed.
There was a problem hiding this comment.
Just what I have observed, but I'll look for a reference. I think it might originate with how FPE is enabled/disabled. On amd64 FPE is always on until you mask it to off and then the mask is reset. Whereas on aarch64 the FPE is off until you explicitly enable it. Aarch64 doesn't have the concept of "masking."
No description provided.