Please use GitHub pull requests for new patches. Avoid migrating existing patches. Phabricator shutdown timeline

[AMDGPU] Add GFX11 ds_bvh_stack_rtn_b32 instruction
ClosedPublic
Actions

Authored by foad on Sep 15 2022, 3:09 AM.

Details

Reviewers

Joe_Nash
piotr
rampitec
arsenm

Group Reviewers

Restricted Project

Commits

rG3822a01e0be2: [AMDGPU] Add GFX11 ds_bvh_stack_rtn_b32 instruction

Diff Detail

Repository: rG LLVM Github Monorepo

Event Timeline

foad created this revision.Sep 15 2022, 3:09 AM

Herald added a project: Restricted Project. · View Herald TranscriptSep 15 2022, 3:09 AM

Herald added subscribers: kosarev, kerbowa, hiraditya and 8 others. · View Herald Transcript

foad requested review of this revision.Sep 15 2022, 3:09 AM

Herald added a project: Restricted Project. · View Herald TranscriptSep 15 2022, 3:09 AM

Herald added subscribers: llvm-commits, wdng. · View Herald Transcript

Harbormaster completed remote builds in B186815: Diff 460346.Sep 15 2022, 4:47 AM

I don't see why this needs manual selection

In D133928#3792158, @arsenm wrote:

I don't see why this needs manual selection

The comments say "tablegen doesn't support matching instructions with multiple outputs". Is that not true? (I'm not the original author of this patch.)

In D133928#3792248, @foad wrote:

In D133928#3792158, @arsenm wrote:

I don't see why this needs manual selection

The comments say "tablegen doesn't support matching instructions with multiple outputs". Is that not true? (I'm not the original author of this patch.)

I missed the second return value somehow

llvm/lib/Target/AMDGPU/DSInstructions.td
289	Does this really have side effects?

This revision is now accepted and ready to land.Sep 15 2022, 6:57 AM

foad added inline comments.Sep 15 2022, 7:29 AM

llvm/lib/Target/AMDGPU/DSInstructions.td
289	It reads and writes LDS in a complicated way. Previously we've used hasSideEffects=1 to model complicated GDS access, e.g. on the DS_ADD/SUB_GS_REG_RTN instructions. Is there a better way? Should we be using MMOs with addrspace 2/3 but no pointer info instead? If so I'd prefer to do that as a follow up.

arsenm added inline comments.Sep 15 2022, 7:31 AM