[PATCH] D22373: [GlobalISel] Introduce a simple instruction selector.

[Eric Christopher via llvm-commits]

echristo added a subscriber: echristo.

================
Comment at: include/llvm/Target/TargetSubtargetInfo.h:93-95
@@ +92,5 @@
+
+  virtual const InstructionSelector *getInstructionSelector() const {
+    return nullptr;
+  }
+
----------------
Do you really intend for this to be a subtarget feature?

================
Comment at: lib/CodeGen/MachineRegisterInfo.cpp:42
@@ +41,3 @@
+
+#ifdef LLVM_BUILD_GLOBAL_ISEL
+  // If Reg was generic and we were tracking a size, we don't need to anymore.
----------------
These ifdefs look like they should be made a bit more generic in some way?

================
Comment at: lib/CodeGen/MachineRegisterInfo.cpp:142-145
@@ -116,2 +141,6 @@
 void MachineRegisterInfo::setSize(unsigned VReg, unsigned Size) {
+  // FIXME: We should be able to check that VReg doesn't have a class, but,
+  // for some reason, that's not true when parsing MIR.
+  //assert(!getRegClassOrRegBank(VReg).is<const TargetRegisterClass *>() &&
+  //       "Can't set the size of a non-generic virtual register");
   getVRegToSize()[VReg] = Size;
----------------
Sadly, don't do this with commented out code. Perhaps something to fix first?

================
Comment at: lib/Target/AArch64/AArch64InstructionSelector.cpp:103
@@ +102,3 @@
+
+  assert(STI.getInstrInfo() && "Subtarget should have TargetInstrInfo!");
+  const TargetInstrInfo &TII = *STI.getInstrInfo();
----------------
These seem a bit ... overly assert-y.

================
Comment at: test/CodeGen/AArch64/GlobalISel/arm64-instructionselect.mir:3
@@ +2,3 @@
+# REQUIRES: global-isel
+
+--- |
----------------
Comment this a bit more please, what you're checking for etc.


https://reviews.llvm.org/D22373