This is an archive of the discontinued LLVM Phabricator instance.

[AMDGPU] Legalize soffset of buffer instruction. Use Waterfall loop logic.
ClosedPublic

Authored by skc7 on Jan 4 2023, 9:48 PM.

Details

Summary

Legalize soffset of buffer instructions. If vgpr is assigned to soffset, use waterfall loop logic to legalize.

Update loadSRsrcFromVGPR method to use for rsrc and soffset operands.

Diff Detail

Unit TestsFailed

Event Timeline

skc7 created this revision.Jan 4 2023, 9:48 PM
Herald added a project: Restricted Project. · View Herald TranscriptJan 4 2023, 9:48 PM
skc7 requested review of this revision.Jan 4 2023, 9:48 PM
Herald added a project: Restricted Project. · View Herald TranscriptJan 4 2023, 9:48 PM
skc7 updated this revision to Diff 486599.Jan 5 2023, 8:58 AM

Use loadMBUFScalarOperandFromVGPR to build waterfall loop for soffset and srsrc.

skc7 updated this revision to Diff 487361.Jan 9 2023, 2:56 AM

Rebase

skc7 retitled this revision from [WIP][AMDGPU] Legalize soffset of buffer instruction. Use Waterfall loop logic. to [AMDGPU] Legalize soffset of buffer instruction. Use Waterfall loop logic..Jan 9 2023, 3:01 AM
skc7 edited the summary of this revision. (Show Details)
skc7 added reviewers: arsenm, alex-t, foad, bcahoon, cdevadas.
skc7 set the repository for this revision to rG LLVM Github Monorepo.

I think this will mishandle the case where both the SRD and the soffset are VGPRs. You need to handle both at the same time in one waterfall loop (this should show up if your tests used a meaningful SRD). You can also just look into the globalisel tests for these intrinsics, they test all the permutations already

llvm/lib/Target/AMDGPU/SIInstrInfo.cpp
5821

Don't understand the isIdenticalTo check, this will miss flag mismatches. Register and subregister equality should be sufficient (maybe it doesn't matter based on context)

llvm/test/CodeGen/AMDGPU/legalize-soffset-mbuf.ll
35

All of these tests are using poison for the SRD, these should use a meaningful value

skc7 updated this revision to Diff 491083.Jan 21 2023, 10:32 AM
skc7 set the repository for this revision to rG LLVM Github Monorepo.

Make changes to legalize soffset and rsrc together.

skc7 added a comment.Jan 21 2023, 10:44 AM

I think this will mishandle the case where both the SRD and the soffset are VGPRs. You need to handle both at the same time in one waterfall loop (this should show up if your tests used a meaningful SRD). You can also just look into the globalisel tests for these intrinsics, they test all the permutations already

Made changes to legalize and use single water fall loop for rsrc and soffset. As per my understanding, SiFixSGPRCopies pass makes call to moveToVALU sequentially (not together for soffset and rsrc) for operands. So we see two water fall loops in the tests.

skc7 updated this revision to Diff 512105.Apr 10 2023, 3:06 AM

Rebase. Update test.

skc7 updated this revision to Diff 513483.Apr 14 2023, 1:46 AM

Port MBUF related tests from global-isel containing all the test combinations.

skc7 added inline comments.Apr 17 2023, 9:06 AM
llvm/test/CodeGen/AMDGPU/legalize-soffset-mbuf.ll
35

These tests are checking if a waterfall loop is created if only soffset is illegal. So, SRD has been made poison.

skc7 updated this revision to Diff 516751.Apr 25 2023, 4:49 AM
skc7 set the repository for this revision to rG LLVM Github Monorepo.

Rebase.

arsenm accepted this revision.Apr 26 2023, 10:19 AM
This revision is now accepted and ready to land.Apr 26 2023, 10:19 AM
This revision was landed with ongoing or failed builds.Apr 27 2023, 7:08 AM
This revision was automatically updated to reflect the committed changes.