[TableGen][GlobalISel] add handling of nested *_SUBREG

[ehjogab]

Summary

When nesting INSERT_SUBREG and EXTRACT_SUBREG, GlobalISelEmitter would
fail to find the register class of the nested node. This patch fixes
that for registers with subregs.

[arsenm]

llvm/test/TableGen/GlobalISelEmitter-nested-subregs.td

34

This is missing the checks?

llvm/utils/TableGen/GlobalISelEmitter.cpp

4954

No else after return

4960

No else after return

[ehjogab]

• add missing check in testcase
• fix lint warnings

[ehjogab]

Oops, had missed that. Thanks!

[ehjogab]

llvm/utils/TableGen/GlobalISelEmitter.cpp

4954

In the original code, there is only an 'if' and no 'else', so I followed the same original structure by just adding the 'else if's. I believe the 'else' case is handled by the code that follow these if statements.

4960

Same as above.

[arsenm]

llvm/utils/TableGen/GlobalISelEmitter.cpp

4954

I don't see what you are saying. There was no else after return pattern here before, and I don't see how this is continuing it. The elses can be removed and this will be a sequence of if () { foo(); return }

[ehjogab]

llvm/utils/TableGen/GlobalISelEmitter.cpp

4954

Okay, I see what you're saying now. I'll fix it right away.

[ehjogab]

Removed else after return

[uabelho]

I submitted for ehjogab.

llvm/test/TableGen/GlobalISelEmitter-nested-subregs.td

+// RUN: llvm-tblgen %s -gen-global-isel -optimize-match-table=false -I %p/../../include -I %p/Common -o - | FileCheck %s
+
+include "llvm/Target/Target.td"
+include "GlobalISelEmitterCommon.td"
+
+let Namespace = "MyTarget" in {
+
+def lo8  : SubRegIndex<8>;
+def hi8  : SubRegIndex<8, 8>;
+def lo16 : SubRegIndex<16>;
+def hi16 : SubRegIndex<16, 16>;
+
+def a0bl : Register<"a0bl">;
+def a0bh : Register<"a0bh">;
+def a0wh : Register<"a0wh">;
+
+} // Namespace = "MyTarget"
+
+def a0wl: RegisterWithSubRegs<"a0", [a0bh, a0bl]> {
+  let SubRegIndices = [hi8, lo8];
+  let CoveredBySubRegs = 1;
+}
+
+def a0: RegisterWithSubRegs<"a0", [a0wh, a0wl]> {
+  let SubRegIndices = [hi16, lo16];
+  let CoveredBySubRegs = 1;
+}
+
+def A0b : RegisterClass<"MyTarget",  [i8],  8, (add a0bl)>;
+def A0w : RegisterClass<"MyTarget", [i16], 16, (add a0wl)>;
+def A0  : RegisterClass<"MyTarget", [i32], 32, (add a0)>;
+
+// CHECK: GIM_CheckNumOperands, /*MI*/0, /*Expected*/2,
+// CHECK-NEXT: GIM_CheckOpcode, /*MI*/0, TargetOpcode::G_ANYEXT,
+// CHECK-NEXT: // MIs[0] dst
+// CHECK-NEXT: GIM_CheckType, /*MI*/0, /*Op*/0, /*Type*/GILLT_s16,
+// CHECK-NEXT: GIM_CheckRegBankForClass, /*MI*/0, /*Op*/0, /*RC*/MyTarget::A0RegClassID,
+// CHECK-NEXT: // MIs[0] src
+// CHECK-NEXT: GIM_CheckType, /*MI*/0, /*Op*/1, /*Type*/GILLT_s8,
+// CHECK-NEXT: // (anyext:{ *:[i16] } i8:{ *:[i8] }:$src)  =>  (EXTRACT_SUBREG:{ *:[i16] } (INSERT_SUBREG:{ *:[i32] } (IMPLICIT_DEF:{ *:[i32] }), A0b:{ *:[i8] }:$src, lo8:{ *:[i32] }), lo16:{ *:[i32] })
+// CHECK-NEXT: GIR_MakeTempReg, /*TempRegID*/0, /*TypeID*/GILLT_s32,
+// CHECK-NEXT: GIR_MakeTempReg, /*TempRegID*/1, /*TypeID*/GILLT_s32,
+// CHECK-NEXT: GIR_BuildMI, /*InsnID*/2, /*Opcode*/TargetOpcode::IMPLICIT_DEF,
+// CHECK-NEXT: GIR_AddTempRegister, /*InsnID*/2, /*TempRegID*/1, /*TempRegFlags*/RegState::Define,
+// CHECK-NEXT: GIR_ConstrainSelectedInstOperands, /*InsnID*/2,
+// CHECK-NEXT: GIR_BuildMI, /*InsnID*/1, /*Opcode*/TargetOpcode::INSERT_SUBREG,
+// CHECK-NEXT: GIR_AddTempRegister, /*InsnID*/1, /*TempRegID*/0, /*TempRegFlags*/RegState::Define,
+// CHECK-NEXT: GIR_AddTempRegister, /*InsnID*/1, /*TempRegID*/1, /*TempRegFlags*/0,
+// CHECK-NEXT: GIR_Copy, /*NewInsnID*/1, /*OldInsnID*/0, /*OpIdx*/1, // src
+// CHECK-NEXT: GIR_AddImm, /*InsnID*/1, /*Imm*/3,
+// CHECK-NEXT: GIR_ConstrainOperandRC, /*InsnID*/1, /*Op*/0, MyTarget::A0RegClassID,
+// CHECK-NEXT: GIR_ConstrainOperandRC, /*InsnID*/1, /*Op*/1, MyTarget::A0RegClassID,
+// CHECK-NEXT: GIR_ConstrainOperandRC, /*InsnID*/1, /*Op*/2, MyTarget::A0bRegClassID,
+// CHECK-NEXT: GIR_BuildMI, /*InsnID*/0, /*Opcode*/TargetOpcode::COPY,
+// CHECK-NEXT: GIR_Copy, /*NewInsnID*/0, /*OldInsnID*/0, /*OpIdx*/0, // dst
+// CHECK-NEXT: GIR_AddTempSubRegister, /*InsnID*/0, /*TempRegID*/0, /*TempRegFlags*/0, MyTarget::lo16,
+// CHECK-NEXT: GIR_EraseFromParent, /*InsnID*/0,
+// CHECK-NEXT: GIR_ConstrainOperandRC, /*InsnID*/0, /*Op*/0, MyTarget::A0wRegClassID,
+// CHECK-NEXT: GIR_ConstrainOperandRC, /*InsnID*/0, /*Op*/1, MyTarget::A0RegClassID,
+def : Pat<(i16 (anyext i8:$src)),
+          (i16 (EXTRACT_SUBREG
+                 (i32 (INSERT_SUBREG
+                        (i32 (IMPLICIT_DEF)),
+                        A0b:$src,
+                        lo8)),
+                 lo16))>;

llvm/utils/TableGen/GlobalISelEmitter.cpp

GlobalISelEmitter::inferRegClassFromPattern(TreePatternNode *N) {
   if (IsRegSequence || InstName == "COPY_TO_REGCLASS") {
     // If we have a COPY_TO_REGCLASS, then we need to handle it specially. It
     // has the desired register class as the first child.
     TreePatternNode *RCChild = N->getChild(IsRegSequence ? 0 : 1);
     if (!RCChild->isLeaf())
       return None;
     return getRegClassFromLeaf(RCChild);
   }
+  if (InstName == "INSERT_SUBREG") {
+    TreePatternNode *Child0 = N->getChild(0);
+    assert(Child0->getNumTypes() == 1 && "Unexpected number of types!");
+    const TypeSetByHwMode &VTy = Child0->getExtType(0);
+    return inferSuperRegisterClassForNode(VTy, Child0, N->getChild(2));
+  }
+  if (InstName == "EXTRACT_SUBREG") {
+    assert(N->getNumTypes() == 1 && "Unexpected number of types!");
+    const TypeSetByHwMode &VTy = N->getExtType(0);
+    return inferSuperRegisterClass(VTy, N->getChild(1));
+  }
 
   // Handle destination record types that we can safely infer a register class
   // from.
   const auto &DstIOperand = Inst.Operands[0];
   Record *DstIOpRec = DstIOperand.Rec;
   if (DstIOpRec->isSubClassOf("RegisterOperand")) {
     DstIOpRec = DstIOpRec->getValueAsDef("RegClass");
     const CodeGenRegisterClass &RC = Target.getRegisterClass(DstIOpRec);