[PATCH] D33596: [globalisel][tablegen] Add support for EXTRACT_SUBREG.

[Ahmed Bougacha via Phabricator via llvm-commits]

ab added a comment.

Can you add a tablegen testcase?



================
Comment at: lib/Target/AArch64/AArch64InstructionSelector.cpp:943-944
 
+    if (DstTy == LLT::scalar(32) && SrcTy == LLT::scalar(64))
+      llvm_unreachable("Tablegen code can import this");
+
----------------
Remove the:

      } else if (DstRC == &AArch64::GPR32RegClass &&
                 SrcRC == &AArch64::GPR64RegClass) {

below instead.


================
Comment at: utils/TableGen/CodeGenRegisters.cpp:923
 
+CodeGenRegisterClass *CodeGenRegisterClass::getMatchingSubClassWithSubRegs(
+    CodeGenRegBank &RegBank, const CodeGenSubRegIndex *SubIdx,
----------------
Looks like we already have an escape hatch in TargetRegisterInfo::getSubClassWithSubReg.  Should we just use that?


================
Comment at: utils/TableGen/CodeGenRegisters.h:374
+                                   const CodeGenSubRegIndex *SubIdx,
+                                   CodeGenRegisterClass **SubRegClass) const;
+
----------------
Return an Optional<std::pair> ?


================
Comment at: utils/TableGen/GlobalISelEmitter.cpp:871
+  const StringRef SymbolicName;
+  // The subregister to extract.
+  const CodeGenSubRegIndex *SubReg;
----------------
Triple-slash.


================
Comment at: utils/TableGen/GlobalISelEmitter.cpp:1625-1630
+      CodeGenRegisterClass *DstRC = nullptr;
+      CodeGenRegisterClass *SrcRC =
+          RC->getMatchingSubClassWithSubRegs(CGRegs, SubIdx, &DstRC);
+      assert(DstRC && "Couldn't find a matching subclass");
+      if (SrcRC != RC)
+        return failedImport("EXTRACT_SUBREG requires an additional COPY");
----------------
At this point, we don't need to compute SrcRC/DstRC, right?  Can you check this when emitting the constraining code?


================
Comment at: utils/TableGen/GlobalISelEmitter.cpp:1817
+
+    if (DefInit *SubRegInit =
+            dyn_cast<DefInit>(Dst->getChild(1)->getLeafValue())) {
----------------
Split the 'if' from the define and early-exit?


================
Comment at: utils/TableGen/GlobalISelEmitter.cpp:1825
+        return failedImport(
+            "COPY_TO_REGCLASS operand #1 isn't a register class");
+
----------------
Stale text?


https://reviews.llvm.org/D33596