This is an archive of the discontinued LLVM Phabricator instance.

Stop traping on sNaN in __builtin_isinf
ClosedPublic

Authored by thopre on Feb 20 2021, 2:17 PM.

Details

Summary

__builtin_isinf currently generates a floating-point compare operation
which triggers a trap when faced with a signaling NaN in StrictFP mode.
This commit uses integer operations instead to not generate any trap in
such a case.

Diff Detail

Event Timeline

thopre created this revision.Feb 20 2021, 2:17 PM
thopre requested review of this revision.Feb 20 2021, 2:17 PM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 20 2021, 2:17 PM
kpn added a comment.Feb 22 2021, 7:30 AM

System/Z's TEST DATA CLASS instruction covers most (all?) of the possible FP value states. You might want to subscribe, or add as a reviewer, jonpa just to make sure everyone stays in sync.

I think we should also add tests for half

jonpa added a comment.Feb 22 2021, 9:51 AM
In D97125#2578853, @kpn wrote:

System/Z's TEST DATA CLASS instruction covers most (all?) of the possible FP value states. You might want to subscribe, or add as a reviewer, jonpa just to make sure everyone stays in sync.

Thanks for keeping me updated on this - I see that you have already included the testFPKind() hook, so I will add these opcodes to SystemZ once this is committed...

thopre updated this revision to Diff 325718.Feb 23 2021, 2:11 AM
  • Add half testcase
  • Fixup existing testcase
thopre added inline comments.Feb 23 2021, 2:13 AM
clang/test/CodeGen/builtin_float_strictfp.c
21

Am I right to think that clang should not use llvm.convert here since it's a constrained operation? Any idea why that happens?

thopre added a comment.Mar 2 2021, 7:17 AM

Ping?

clang/test/CodeGen/builtin_float_strictfp.c
21

Nvm, that's for when fp16 is not supported. I'm guessing this ought to not throw an exception.

mibintc accepted this revision.Mar 2 2021, 7:32 AM
This revision is now accepted and ready to land.Mar 2 2021, 7:32 AM
This revision was landed with ongoing or failed builds.Mar 2 2021, 7:55 AM
This revision was automatically updated to reflect the committed changes.
thopre reopened this revision.Mar 4 2021, 7:30 AM
This revision is now accepted and ready to land.Mar 4 2021, 7:30 AM
thopre updated this revision to Diff 328170.Mar 4 2021, 7:31 AM

Fix isfinite logic

thopre updated this revision to Diff 328171.Mar 4 2021, 7:40 AM

Add isfinite testcases

thopre requested review of this revision.Mar 4 2021, 8:55 AM

Requesting review since the logic has changed. This time I've also tested isfinite against glibc's result. All looks good.

Requesting review since the logic has changed. This time I've also tested isfinite against glibc's result. All looks good.

Ping?

mibintc accepted this revision.Mar 15 2021, 5:46 AM
This revision is now accepted and ready to land.Mar 15 2021, 5:46 AM
This revision was automatically updated to reflect the committed changes.