[PATCH] D31761: [globalisel][tablegen] Revise API for ComplexPattern operands to improve flexibility.
Ahmed Bougacha via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Mon Apr 10 13:16:45 PDT 2017
ab added a comment.
I'm slightly worried that this makes the selector functions a bit harder to read, but this seems like a nice simplification otherwise.
One question: does this prevent us from ever selecting operands in-place? The previous implementation seems amenable to that (by only assigning the complex-pattern MO instead of adding them all to a new MI). But now we'd be forced to always append operands, right?
This also means that we can remove MO_Placeholder, yes?
================
Comment at: include/llvm/CodeGen/GlobalISel/InstructionSelector.h:80
+ typedef std::function<void(MachineInstrBuilder &)> ComplexRendererFn;
+ typedef Optional<ComplexRendererFn> OptionalComplexRendererFn;
+
----------------
The Optional seems a bit redundant: std::function is already optional. Can we return a {} default?
================
Comment at: test/TableGen/GlobalISelEmitter-GIRule.td:70
// CHECK-NEXT: ((/* src2 */ (MRI.getType(MI0.getOperand(2).getReg()) == (LLT::scalar(32))) &&
-// CHECK-NEXT: (selectComplexPattern(MI0.getOperand(2), TempOp0, TempOp1)))) &&
+// CHECK-NEXT: ((Renderer0 = selectComplexPattern(MI0.getOperand(2)), Renderer0.hasValue())))) &&
// CHECK-NEXT: ((/* src3 */ (MRI.getType(MI0.getOperand(3).getReg()) == (LLT::scalar(32))) &&
----------------
What about:
((Renderer0 = selectComplexPattern(MI0.getOperand(2))))
I found the ',' more surprising than the '='.
https://reviews.llvm.org/D31761
More information about the llvm-commits
mailing list