[PATCH] D134423: [AMDGPU] Fix vgpr2sgpr copy analysis to check scalar operands of buffer instructions use scalar registers.

Jay Foad via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Nov 9 06:45:11 PST 2022


foad added a comment.

Hi @skc7, I've looked at the failure in your new test `@llvm_amdgcn_raw_buffer_load_f32`. I think this is a case that the AMDGPU backend has never handled properly, even before @alex-t rewrote SIFixSGPRCopies in D128252 <https://reviews.llvm.org/D128252>.

You have a BUFFER_LOAD_DWORD_OFFEN instruction with a divergent (vgpr) value for the `$soffset` operand. SIInstrInfo::legalizeOperands ought to fix this somehow, but it does not. See the FIXME here:

  // Legalize MUBUF* instructions.
  int RsrcIdx =
      AMDGPU::getNamedOperandIdx(MI.getOpcode(), AMDGPU::OpName::srsrc);
  if (RsrcIdx != -1) {
    // We have an MUBUF instruction
    MachineOperand *Rsrc = &MI.getOperand(RsrcIdx);
    unsigned RsrcRC = get(MI.getOpcode()).OpInfo[RsrcIdx].RegClass;
    if (RI.getCommonSubClass(MRI.getRegClass(Rsrc->getReg()),
                             RI.getRegClass(RsrcRC))) {
      // The operands are legal.
      // FIXME: We may need to legalize operands besides srsrc.
      return CreatedBB;
    }

It only expects to legalize the `$srsrc` operand, which it will do by creating a waterfall loop. To legalize the `$soffset` operand it could either do something clever with addressing, like adding it into the `$offset` operand (buffer addressing is complicated so that might not be valid), or it could create another waterfall loop. Or it could use readfirstlane, which is a bit of a cop-out but would at least avoid crashing the compiler for now.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D134423/new/

https://reviews.llvm.org/D134423



More information about the llvm-commits mailing list