[PATCH] D43042: [MachineOperand][Target] Add target option to disable setting MachineOperand::isRenamable

[Quentin Colombet via Phabricator via llvm-commits]

qcolombet added a comment.

Hi Geoff,

I believe this patch is going in the right direction of allowing targets to safely bail out from the optimization.
Now, like Fiona said, the constraints we want to express are either tied to instructions (encoding constraint not modeled in TableGen) or operands (ABI) but we only carry the information around in the machine operands. This is a problem, because each time we transfer operands, we need to make sure the operands inherit what the instructions want.

The setDesc part of the patch goes in that direction, but is not enough.
For instance, in expand post ra pseudo, we transfer the implicit operands of a copy to a lowered version of that same copy. By doing that we don't reset the renamable flags on the operand and potentially we would end up with renamable operands on a hasExtraXXXRegAllocReq instruction. (The setDesc fix wouldn't be enough since the transfer happens after the creation.)

One way we could fix that is by making isRenamable check both the flag on the operand and the hasExtraXXX flag of its parent (if any).
The plus side is that we wouldn't need to change setDesc.

What do you think?

Cheers,
-Quentin


Repository:
  rL LLVM

https://reviews.llvm.org/D43042