This is an archive of the discontinued LLVM Phabricator instance.

[AMDGPU] gfx11 Select on Buffer Atomic FAdd Rtn type
ClosedPublic

Authored by Joe_Nash on Jun 20 2022, 7:23 AM.

Diff Detail

Event Timeline

Joe_Nash created this revision.Jun 20 2022, 7:23 AM
Herald added a project: Restricted Project. · View Herald TranscriptJun 20 2022, 7:23 AM
Joe_Nash requested review of this revision.Jun 20 2022, 7:23 AM
Herald added a project: Restricted Project. · View Herald TranscriptJun 20 2022, 7:23 AM
Joe_Nash added reviewers: Restricted Project, foad, rampitec, Petar.Avramovic.Jun 20 2022, 7:24 AM
rampitec accepted this revision.Jun 20 2022, 10:43 AM
This revision is now accepted and ready to land.Jun 20 2022, 10:43 AM
foad added a comment.Jun 21 2022, 3:10 AM

What about globalisel?

Joe_Nash updated this revision to Diff 439090.Jun 22 2022, 10:37 AM

added globalisel support

foad added inline comments.Jun 23 2022, 3:45 AM
llvm/lib/Target/AMDGPU/AMDGPUGlobalISelUtils.cpp
76

No else after return.

llvm/lib/Target/AMDGPU/SIISelLowering.cpp
12638

Is this change intentional? What is the effect of it?

Joe_Nash updated this revision to Diff 439399.Jun 23 2022, 7:59 AM
Joe_Nash marked an inline comment as done.

removed return after else

llvm/lib/Target/AMDGPU/SIISelLowering.cpp
12638

I think this is NFC. hasAtomicFaddInsts definition was changed in D124538. There are no current subtargets that have FeatureAtomicFaddRtnInsts but not FeatureAtomicFaddNoRtnInsts. So they are both correct, but this is more precise. I would need @Petar.Avramovic to confirm.

foad accepted this revision.Jun 23 2022, 8:01 AM
foad added inline comments.
llvm/lib/Target/AMDGPU/SIISelLowering.cpp
12638

this is more precise

Fair enough.

This revision was landed with ongoing or failed builds.Jun 23 2022, 8:35 AM
This revision was automatically updated to reflect the committed changes.