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

[Dean Michael Berris via llvm-commits]

dberris added inline comments.

================
Comment at: include/llvm/CodeGen/GlobalISel/InstructionSelector.h:33-36
@@ +32,6 @@
+  ///
+  /// \returns whether selection succeeded.
+  /// \pre  I.getParent() && I.getParent()->getParent()
+  /// \post if returns true: !isPreISelGenericOpcode(I.getOpcode())
+  virtual bool select(MachineInstr &I) const = 0;
+
----------------
I'm thinking about this from the xray point of view, so let me expound a little.

Consider when for example we see a pseudo instruction like: PATCHABLE_FUNCTION_ENTER or PATCHABLE_RET -- right now it's selected on a per-platform basis when lowering. If we were to change this to become generic instructions instead (i.e. not pseudo instructions), we can then select the appropriate lowering by default (say, for PATCHABLE_FUNCTION_ENTER it would just disappear, and PATCHABLE_RET will just unpack the machine opcodes it wraps), and specialise on a per-platform basis.

We're thinking of cases where say we have other instructions that we'd like to conditionally prepend or append with the appropriate sleds, without having to explicitly mark them at a higher level too (for example, looking at tail exits, sibling exits, or even before/after calls to a whitelist of functions).

Maybe the question is whether XRay sled emission should happen at selection time, or stay in lowering as it is today?

================
Comment at: lib/CodeGen/GlobalISel/InstructionSelect.cpp:63-64
@@ +62,4 @@
+      // obvious how, esp. considering select() can insert after MI.
+    }
+  }
+
----------------
I think adding more nuance to it might be useful -- say, instead of just 'cannot select an instruction' it could be that there might be cases that the instruction was malformed (some operands were outside some range, or some invariant was violated -- i.e. this should only appear once in a function but it appears multiple times). Maybe that's a legalisation issue though and the error might be better there. Maybe it'd be fine to extend later when we have more concrete cases where it matters.


https://reviews.llvm.org/D22373