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

[Daniel Sanders via Phabricator via llvm-commits]

dsanders added a comment.

In https://reviews.llvm.org/D30089#695620, @qcolombet wrote:

> > What would be the benefit of a placeholder instruction over placeholder operands?
>
> I expect the placeholder reset to be something just a single assignment of the style size of array operand = 0. The operands remain as they are, they are just not accessed. In other words, no constructor call.


The constructor call comes from the need to call addOperand(). If we reset the operand list for the placeholder instruction on each call to selectImpl() then we would need something like:

  PlaceholderInsn.addOperand(MachineOperand::CreatePlaceholder());
  PlaceholderInsn.addOperand(MachineOperand::CreatePlaceholder());
  if (matches && selectComplexPattern(PlaceholderInsn.getOperand(0), PlaceholderInsn.getOperand(1))) {
    BuildMI(...).add(PlaceholderInsn.getOperand(0)).add(PlaceholderInsn.getOperand(1));
    ...
  }
  PlaceholderInsn.resetOperands();

Each call to addOperand() involves a placement new operator which uses the copy-constructor and therefore pays the same overhead as the current local variable approach.

I think your main concern is that the overhead is unnecessarily paid on each call to selectImpl(). I've updated the patch with a version that only pays the constructor overhead once (during <Target>InstructionSelection's constructor). Does that resolve your concerns?


https://reviews.llvm.org/D30089