[PATCH] D26878: [GlobalISel] Add tentative Selector-emitted tablegen backend.

[Daniel Sanders via Phabricator via llvm-commits]

dsanders added inline comments.


================
Comment at: include/llvm/Target/TargetGlobalISel.td:23
+  Instruction I = i;
+}
+
----------------
qcolombet wrote:
> Just a forward looking thought. Do you think that we would need to support 1 to many patterns?
> E.g., what will be the strategy for BUILD_VECTOR and the kind?
> 
> I am starting to think that we are defining patterns, just target independent ones.
We haven't used it yet so I can't be sure but G_SEQUENCE seems like the gMIR equivalent of BUILD_VECTOR. Is that right?


================
Comment at: utils/TableGen/GlobalISelEmitter.cpp:99
+
+struct MatchAction {
+  virtual ~MatchAction() {}
----------------
ab wrote:
> dsanders wrote:
> > This is a nitpick but I think we should keep Matchers and Actions in separate class hierarchies for type checking purposes. They don't share much implementation and it doesn't make sense to put MatchOpcode in Matcher::Actions or MutateOpcode in Matcher::Matchers
> Fair enough; split it into 'Matcher' and 'MatchAction', and renamed 'Matcher' to 'MatcherEmitter'.  Better names welcome ;)
For 'Matcher', could you elaborate that it's matching properties of an operand (OperandPropertyMatcher?). That way we won't have to rename it when we add additional levels of matching. For example, I've added an InstructionMatcher in my current WIP and I suspect I'll find an OperandMatcher useful now that we can have multiple matchers on a single operand.


https://reviews.llvm.org/D26878