This is an archive of the discontinued LLVM Phabricator instance.

[AMDGPU] Use new opcode for indexed vgpr reads
ClosedPublic

Authored by foad on Nov 19 2021, 2:45 AM.

Details

Summary

Introduce V_MOV_B32_indirect_read for indexed vgpr reads
(and rename the old V_MOV_B32_indirect to
V_MOV_B32_indirect_write) so they can be unambiguously
distinguished from regular V_MOV_B32_e32. Previously they
were distinguished by looking for extra implicit operands
but this is fragile because regular moves sometimes have
extra implicit operands too:

  • either by accident, when instructions end up with duplicate implicit operands (see e.g. D100939)
  • or by design, when SIInstrInfo::copyPhysReg breaks a multi-dword copy into individual subreg mov instructions and adds implicit operands for the super-register.

The effect of this is that SIInstrInfo::isFoldableCopy can
be simplified and identifies more foldable copies. The test
diffs show that more immediate 0 values have been folded as
inline operands.

SIInstrInfo::isReallyTriviallyReMaterializable could
probably be simplified too but that is not part of this
patch.

Diff Detail

Event Timeline

foad created this revision.Nov 19 2021, 2:45 AM
foad requested review of this revision.Nov 19 2021, 2:45 AM
Herald added a project: Restricted Project. · View Herald TranscriptNov 19 2021, 2:45 AM
arsenm accepted this revision.Nov 19 2021, 5:04 AM

LGTM, I've meant to do this before

llvm/lib/Target/AMDGPU/VOP1Instructions.td
879–880

It's a weird accident we can actually use PseudoInstExpansion here since we only need to deal with the one encoding

This revision is now accepted and ready to land.Nov 19 2021, 5:04 AM
arsenm added inline comments.Nov 19 2021, 5:06 AM
llvm/lib/Target/AMDGPU/VOP1Instructions.td
870–871

Probably should be a separate patch, but should these have implicit m0 reads (and maybe hasSideEffects = 1?)

This revision was automatically updated to reflect the committed changes.
foad added inline comments.Nov 19 2021, 5:53 AM
llvm/lib/Target/AMDGPU/VOP1Instructions.td
870–871

Probably should be a separate patch, but should these have implicit m0 reads

D114239.

and maybe hasSideEffects = 1?

That doesn't seem necessary. Do these instructions have any side effects that aren't adequately modelled by the implicit operands?

llvm/lib/Target/AMDGPU/VOP1Instructions.td