This is an archive of the discontinued LLVM Phabricator instance.

AtomicExpand: Fix expanding atomics into unconstrained FP in strictfp functions
ClosedPublic

Authored by arsenm on Jul 11 2023, 11:26 AM.

Details

Summary

Ideally the normal fadd/fmin/fmax this was creating would fail the verifier.
It's probably also necessary to force off FP exception handlers in the cmpxchg
loop but we don't have a generic way to do that now.

Note strictfp builder is broken in the minnum/maxnum case

Diff Detail

Event Timeline

arsenm created this revision.Jul 11 2023, 11:26 AM
arsenm requested review of this revision.Jul 11 2023, 11:26 AM
Herald added a project: Restricted Project. · View Herald TranscriptJul 11 2023, 11:26 AM
Herald added a subscriber: wdng. · View Herald Transcript
kpn added a comment.Jul 11 2023, 11:35 AM

We're going to run into this in other cases as well.

Can these functions be called for instructions that are in blocks that are not currently in a function? I seem to remember we ran into that odd case sometime back.

I'll have to trust you on the AMDGPU and X86 instructions.

We're going to run into this in other cases as well.

Can these functions be called for instructions that are in blocks that are not currently in a function? I seem to remember we ran into that odd case sometime back.

No, this is for straightforward lowering. I was debating whether the IRBuilder constructor should take care of this for you

kpn accepted this revision.Jul 11 2023, 11:39 AM

OK, LGTM.

This revision is now accepted and ready to land.Jul 11 2023, 11:39 AM
jyknight added inline comments.Jul 11 2023, 12:54 PM
llvm/lib/CodeGen/AtomicExpandPass.cpp
1582

I'd reword: "If FP exceptions are observable".
And, FTR, GCC does already do this (and it's required by the spec). So we should actually fix that at some point. :)

arsenm added inline comments.Jul 11 2023, 4:10 PM
llvm/lib/CodeGen/AtomicExpandPass.cpp
1582