This is an archive of the discontinued LLVM Phabricator instance.

[AMDGPU] Add GFX11 ds_bvh_stack_rtn_b32 instruction
ClosedPublic

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

Diff Detail

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
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

I don't see why this needs manual selection

foad added a comment.Sep 15 2022, 6:54 AM

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.)

arsenm accepted this revision.Sep 15 2022, 6:57 AM

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
llvm/lib/Target/AMDGPU/DSInstructions.td
289

Yes, MMOs would be better

This revision was automatically updated to reflect the committed changes.