[PATCH] D30089: [globalisel][tblgen] Add support for ComplexPatterns

[Quentin Colombet via Phabricator via llvm-commits]

qcolombet added a comment.

One remark: I can see how a placeholder operand can help detecting errors (like we forgot to update one of the operand), however, I think it gets in the way of efficiency.
Indeed, I imagine that each time a complex pattern will fail half way through, we will have to reset all the operands to placeholder instead of just keeping them in the state they are.

What I am saying is that we may not introduce a placeholder operand at all. We should have a placeholder opcode though that reset the operands array to 0 (without freeing memory).



================
Comment at: lib/Target/AArch64/AArch64InstructionSelector.cpp:1240
+  unsigned ShVal = AArch64_AM::getShifterImm(AArch64_AM::LSL, ShiftAmt);
+  Result1 = MachineOperand::CreateImm(Immed);
+  Result2 = MachineOperand::CreateImm(ShVal);
----------------
dsanders wrote:
> qcolombet wrote:
> > Can't we modify the existing placeholder instead of creating a temporary object and then use the copy constructor?
> I think we should be able to use ChangeToImmediate(), but I haven't tried it yet. The bit that worried me about doing that was having stale values in members that aren't affected by the ChangeTo*() functions. Most of them look ok (since they only matter for registers and ChangeToRegister() does reset them) but I'm not sure about MachineOperand::ParentMI.
That should be easy to check/fix :).


https://reviews.llvm.org/D30089