This is an archive of the discontinued LLVM Phabricator instance.

AtomicExpand: Support cmpxchg expansion for small FP types
ClosedPublic

Authored by arsenm on Sep 23 2022, 1:18 PM.

Details

Summary

Handles f16 atomics for AMDGPU.

Diff Detail

Event Timeline

arsenm created this revision.Sep 23 2022, 1:18 PM
Herald added a project: Restricted Project. · View Herald TranscriptSep 23 2022, 1:18 PM
arsenm requested review of this revision.Sep 23 2022, 1:18 PM
Herald added a project: Restricted Project. · View Herald TranscriptSep 23 2022, 1:18 PM
Herald added a subscriber: wdng. · View Herald Transcript
foad added inline comments.Sep 26 2022, 1:00 AM
llvm/lib/CodeGen/AtomicExpandPass.cpp
617

Spanish Inquisition bug: "four fields"

Can you add a simple test to llvm/test/Transforms/AtomicExpand/SPARC/partword.ll for fadd, since that's where the other "generic" tests of this code live.

llvm/lib/CodeGen/AtomicExpandPass.cpp
824

Same code as section above. Merge the two and update comment.

843–848

This shifted value isn't actually used for the existing min/max/umin/umax -- nor the new fadd/fsub/fmin/fmax support.

Perhaps we should just avoid creating it when it's unneeded?

Can you add a simple test to llvm/test/Transforms/AtomicExpand/SPARC/partword.ll for fadd, since that's where the other "generic" tests of this code live.

(Actually I'm not sure if 'half' can be codegened at all on that target...so maybe that's not possible. But if it is it would be nice.)

arsenm marked an inline comment as done.Sep 29 2022, 1:45 PM
arsenm added inline comments.
llvm/lib/CodeGen/AtomicExpandPass.cpp
843–848

Fixed this in D134903

arsenm updated this revision to Diff 464018.Sep 29 2022, 1:57 PM

Address comments

jyknight accepted this revision.Nov 9 2022, 7:00 AM
This revision is now accepted and ready to land.Nov 9 2022, 7:00 AM