AMDGPU: Skip folding REG_SEQUENCE if found unknown regclasses for its users.

[arsenm]

What's the reasoning for this? Won't this break chains of REG_SEQUENCE? In particular I do expect to see some REG_SEQUENCE (REG_SEQUENCE), (REG_SEQUENCE) patterns

llvm/lib/Target/AMDGPU/SIInstrInfo.cpp

4758 ↗

(On Diff #412398)

isUnknownRegClass?

[foad]

llvm/lib/Target/AMDGPU/SIInstrInfo.cpp

4761 ↗

(On Diff #412398)

Why do you need to test isVariadic here()? Surely as long as you compare OpIdx against Desc.getNumOperands() (not MI.getNumOperands()) you're OK?

[cdevadas]

In D120813#3354292, @arsenm wrote:

What's the reasoning for this? Won't this break chains of REG_SEQUENCE? In particular I do expect to see some REG_SEQUENCE (REG_SEQUENCE), (REG_SEQUENCE) patterns

Currently skipping it to avoid a crash that occurred with REG_SEQUENCE(REG_SEQUENCE) while trying to look for the regclass @OpIdx from the UseMI's MCInstrDesc.
These special cases involving target-independent and/or variadic instructions should be handled separately.
This folding will take place only when the regclass at OpIdx is a vectorSuperClass https://github.com/llvm/llvm-project/blob/main/llvm/lib/Target/AMDGPU/SIFoldOperands.cpp#L1603
Currently, we don't select any AV operands and I guess it is ok to skip them.

llvm/lib/Target/AMDGPU/SIInstrInfo.cpp

4758 ↗

(On Diff #412398)

Will use

4761 ↗

(On Diff #412398)

No, OpIdx >= Desc.getNumOperands() didn't hold true when I checked the case REG_SEQUENCE (REG_SEQUENCE). Is it because the passed Op index is already adjusted from UseMI->getOperand(0)?

[cdevadas]

Used a better name for the function.

[rampitec]

llvm/lib/Target/AMDGPU/SIInstrInfo.cpp

4761 ↗

(On Diff #412398)

Even variadic instructions shall have proper operand description before the variadic part starts.

[cdevadas]

Removed variadic instruction check.
Desc.OpInfo[OpIdx].RegClass == -1 will catch those invalid cases.

[foad]

llvm/lib/Target/AMDGPU/SIInstrInfo.cpp

4760 ↗

(On Diff #412876)

Just use MI.getDesc()?

[cdevadas]

Addressed Jay's suggestion.

[foad]

llvm/lib/Target/AMDGPU/SIFoldOperands.cpp

1602

You could just use TargetInstrInfo::getRegClass(InstDesc, OpIdx, TRI, MF) instead of TRI->getRegClass and it would do all these checks for you already.

[cdevadas]

llvm/lib/Target/AMDGPU/SIFoldOperands.cpp

1602

We can't use that equivalent of getRegClass here. The call to adjustAllocatableRegClass inside it would change AV classes to V thereby disabling the folding in almost all valid cases.

[foad]

llvm/lib/Target/AMDGPU/SIFoldOperands.cpp

1602

The call to adjustAllocatableRegClass is in the AMDGPU-specific getOpRegClass, not in the generic getRegClass.

[cdevadas]

llvm/lib/Target/AMDGPU/SIFoldOperands.cpp

1602

@foad thanks for pointing that generic function.

We have some variants in SIInstrInfo.cpp to get regclass value from InstDesc.
I initially tried to use SIInstrInfo::getOpRegClass(const MachineInstr &MI, unsigned OpNo) and found the adjustAllocatableRegClass calls inside it prevented the folding for some tests involving loads and stores.

But SIInstrInfo::getRegClass(InstDesc, OpIdx, TRI, MF) seems to be working. Though it also has a call to adjustAllocatableRegClass, it is more restrained and appeared to be not missing any folding opportunity for loads/stores that failed earlier with the other variant.
I am going to use this function. Will update the patch.

[cdevadas]

Used existing 'SIInstrInfo::getRegClass' function to get the RC at OpIdx.

llvm/lib/Target/AMDGPU/SIFoldOperands.cpp

while (UseMI->isCopy() && !Op->getSubReg()) {
     Op = &*MRI->use_nodbg_begin(Reg);
     UseMI = Op->getParent();
   }
 
   if (Op->getSubReg())
     return false;
 
   unsigned OpIdx = Op - &UseMI->getOperand(0);
   const MCInstrDesc &InstDesc = UseMI->getDesc();
-  if (!TRI->isVectorSuperClass(
-          TRI->getRegClass(InstDesc.OpInfo[OpIdx].RegClass)))
+  const TargetRegisterClass *OpRC =
+      TII->getRegClass(InstDesc, OpIdx, TRI, *MI.getMF());
+  if (!OpRC || !TRI->isVectorSuperClass(OpRC))
     return false;
 
   const auto *NewDstRC = TRI->getEquivalentAGPRClass(MRI->getRegClass(Reg));
   auto Dst = MRI->createVirtualRegister(NewDstRC);
   auto RS = BuildMI(*MI.getParent(), MI, MI.getDebugLoc(),
                     TII->get(AMDGPU::REG_SEQUENCE), Dst);
 
   for (unsigned I = 0; I < Defs.size(); ++I) {

llvm/test/CodeGen/AMDGPU/skip-fold-regsequence.mir

+# NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py
+# RUN: llc -mtriple=amdgcn-amd-amdhsa -mcpu=gfx90a -verify-machineinstrs -run-pass si-fold-operands -o - %s | FileCheck -check-prefix=GCN %s
+
+# Skip folding a REG_SEQUENCE to its user when the regclasses for the user operands can't be
+# fully determined from the instruction description.
+---
+name:            regsequence_with_regsequence_use_op
+tracksRegLiveness: true
+body:             |
+  bb.0:
+    liveins: $agpr0, $agpr1
+
+    ; GCN-LABEL: name: regsequence_with_regsequence_use_op
+    ; GCN: liveins: $agpr0, $agpr1
+    ; GCN-NEXT: {{  $}}
+    ; GCN-NEXT: [[COPY:%[0-9]+]]:vgpr_32 = COPY $agpr0
+    ; GCN-NEXT: [[COPY1:%[0-9]+]]:vgpr_32 = COPY $agpr1
+    ; GCN-NEXT: [[REG_SEQUENCE:%[0-9]+]]:vreg_64_align2 = REG_SEQUENCE [[COPY]], %subreg.sub0, [[COPY1]], %subreg.sub1
+    ; GCN-NEXT: [[DEF:%[0-9]+]]:vgpr_32 = IMPLICIT_DEF
+    ; GCN-NEXT: [[REG_SEQUENCE1:%[0-9]+]]:vreg_96_align2 = REG_SEQUENCE killed [[REG_SEQUENCE]], %subreg.sub0_sub1, killed [[DEF]], %subreg.sub2
+    ; GCN-NEXT: S_ENDPGM 0, implicit [[REG_SEQUENCE1]]
+    %0:vgpr_32 = COPY $agpr0
+    %1:vgpr_32 = COPY $agpr1
+    %2:vreg_64_align2 = REG_SEQUENCE %0:vgpr_32, %subreg.sub0, %1:vgpr_32, %subreg.sub1
+    %3:vgpr_32 = IMPLICIT_DEF
+    %4:vreg_96_align2 = REG_SEQUENCE killed %2:vreg_64_align2, %subreg.sub0_sub1, killed %3:vgpr_32, %subreg.sub2
+    S_ENDPGM 0, implicit %4
+...
+---
+name:            insert_subreg_with_regsequence_use_op
+tracksRegLiveness: true
+body:             |
+  bb.0:
+    liveins: $agpr0, $agpr1
+
+    ; GCN-LABEL: name: insert_subreg_with_regsequence_use_op
+    ; GCN: liveins: $agpr0, $agpr1
+    ; GCN-NEXT: {{  $}}
+    ; GCN-NEXT: [[COPY:%[0-9]+]]:vgpr_32 = COPY $agpr0
+    ; GCN-NEXT: [[COPY1:%[0-9]+]]:vgpr_32 = COPY $agpr1
+    ; GCN-NEXT: [[REG_SEQUENCE:%[0-9]+]]:vreg_64_align2 = REG_SEQUENCE [[COPY]], %subreg.sub0, [[COPY1]], %subreg.sub1
+    ; GCN-NEXT: S_NOP 0, implicit-def %3
+    ; GCN-NEXT: [[INSERT_SUBREG:%[0-9]+]]:vreg_64_align2 = INSERT_SUBREG %3, [[REG_SEQUENCE]], %subreg.sub0_sub1
+    ; GCN-NEXT: S_ENDPGM 0, implicit [[INSERT_SUBREG]]
+    %0:vgpr_32 = COPY $agpr0
+    %1:vgpr_32 = COPY $agpr1
+    %2:vreg_64_align2 = REG_SEQUENCE %0:vgpr_32, %subreg.sub0, %1:vgpr_32, %subreg.sub1
+    S_NOP 0, implicit-def %3:vreg_64_align2
+    %4:vreg_64_align2 = INSERT_SUBREG %3, %2, %subreg.sub0_sub1
+    S_ENDPGM 0, implicit %4
+...