This is an archive of the discontinued LLVM Phabricator instance.

[AMDGPU] Add llvm.amdgcn.global.load.lds intrinsic
ClosedPublic

Authored by rampitec on May 9 2022, 4:35 PM.

Diff Detail

Event Timeline

rampitec created this revision.May 9 2022, 4:35 PM
Herald added a project: Restricted Project. · View Herald TranscriptMay 9 2022, 4:35 PM
rampitec requested review of this revision.May 9 2022, 4:35 PM
Herald added a project: Restricted Project. · View Herald TranscriptMay 9 2022, 4:35 PM
Herald added a subscriber: wdng. · View Herald Transcript
rampitec updated this revision to Diff 429281.May 13 2022, 10:11 AM

Return false from getMemOperandsWithOffsetWidth() instead of checking mem operand.

rampitec updated this revision to Diff 429852.May 16 2022, 2:27 PM

Changed asserts to cannot select.

arsenm accepted this revision.May 16 2022, 2:39 PM

Typo amdgc. in description

llvm/include/llvm/IR/IntrinsicsAMDGPU.td
1780

Does this really need the offset parameter? It's not a buffer intrinsic with the funny different swizzling behavior between scalar and vector, so can't we just match this from the base pointer?

llvm/lib/Target/AMDGPU/SIISelLowering.cpp
1314–1315

I guess this API just isn't strong enough to handle 2 MMOs

This revision is now accepted and ready to land.May 16 2022, 2:39 PM
rampitec retitled this revision from [AMDGPU] Add llvm.amdgc.global.load.lds intrinsic to [AMDGPU] Add llvm.amdgcn.global.load.lds intrinsic.May 16 2022, 2:40 PM
rampitec added inline comments.May 16 2022, 2:45 PM
llvm/include/llvm/IR/IntrinsicsAMDGPU.td
1780

This has 2 separate offsets. VGPR offset is applied to the global memory only, while immediate offset applies to both global and LDS. To have it matching first pointer shall be (g_ptr + const) and the second (l_ptr + const) which we cannot guarantee to preserve.

llvm/lib/Target/AMDGPU/SIISelLowering.cpp
1314–1315

Yes, so it is only for the selection time and later I split it.

arsenm added inline comments.May 16 2022, 2:50 PM
llvm/include/llvm/IR/IntrinsicsAMDGPU.td
1780

Probably should comment offset behavior here

rampitec updated this revision to Diff 429867.May 16 2022, 2:52 PM
rampitec marked 2 inline comments as done.

Added comment about offset operand.

arsenm added inline comments.May 16 2022, 3:00 PM
llvm/include/llvm/IR/IntrinsicsAMDGPU.td
1780

You could just not apply the offset if the same constant wasn't used on both pointers. If it's going to be in the intrinsic, there should probably be an optimization to try to put the immediate in the offset field

rampitec added inline comments.May 16 2022, 3:03 PM
llvm/include/llvm/IR/IntrinsicsAMDGPU.td
1780

It should fail to match too often. It is easier to give explicit control to the user. Especially because that is how it is supposed to be used, with the incremental stride by dword.

rampitec updated this revision to Diff 430159.May 17 2022, 12:19 PM

Fixed rebase artifact with misplaced comment.

This revision was landed with ongoing or failed builds.May 17 2022, 12:35 PM
This revision was automatically updated to reflect the committed changes.