[PATCH] D31761: [globalisel][tablegen] Revise API for ComplexPattern operands to improve flexibility.

[Daniel Sanders via Phabricator via llvm-commits]

dsanders added a comment.

Forgot to reply to the inline comments



================
Comment at: include/llvm/CodeGen/GlobalISel/InstructionSelector.h:80
+  typedef std::function<void(MachineInstrBuilder &)> ComplexRendererFn;
+  typedef Optional<ComplexRendererFn> OptionalComplexRendererFn;
+
----------------
ab wrote:
> The Optional seems a bit redundant: std::function is already optional.  Can we return a {} default?
I think that will work. I'll give it a try.


================
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))) &&
----------------
ab wrote:
> What about:
> 
>         ((Renderer0 = selectComplexPattern(MI0.getOperand(2))))
> 
> I found the ',' more surprising than the '='.
Assuming the std::function change above works out, this change will be easy to make.


https://reviews.llvm.org/D31761