Page MenuHomePhabricator

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

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

Diff Detail

Event Timeline

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

What about globalisel?

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

added globalisel support

foad added inline comments.Thu, Jun 23, 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.Thu, Jun 23, 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.Thu, Jun 23, 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.Thu, Jun 23, 8:35 AM
This revision was automatically updated to reflect the committed changes.