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

[Tim Northover via llvm-commits]

t.p.northover added inline comments.

================
Comment at: lib/Target/AArch64/AArch64InstructionSelector.cpp:135
@@ +134,3 @@
+
+    const unsigned DefSize = getRegSize(MRI, DefReg);
+    assert(DefSize && "Virtual register doesn't have a size");
----------------
This can probably be based on `I.getType()` rather than singling out some particular register.

================
Comment at: lib/Target/AArch64/AArch64InstructionSelector.cpp:143
@@ +142,3 @@
+
+    I.setDesc(TII.get(NewOpc));
+    // FIXME: Should the type be always reset in setDesc? If we move the type
----------------
>From here on seems like it would be better off outside the switch. It applies to the vast majority of instructions.

================
Comment at: lib/Target/AArch64/AArch64InstructionSelector.cpp:155
@@ +154,3 @@
+    // number and order of operands is the same pre- and post-isel.
+    for (unsigned OpI = 0, OpE = I.getNumOperands(); OpI != OpE; ++OpI) {
+      MachineOperand &MO = I.getOperand(OpI);
----------------
Do we have a policy on implicit ops on generic `MachineInstrs`? I'm guessing we don't expect there to be any, but if not we might want `getNumExplicitOperands` instead.


https://reviews.llvm.org/D22373