llvm dev - Dec 2018 - CloneMachineInstr and register ties

Hi,

Working on a new pass for a project based on LLVM 4.0.1 I tried to clone 
a machine instruction using MachineFunction::CloneMachineInstr. Its 
documentation reads
> /// CloneMachineInstr - Create a new MachineInstr which is a copy of the
> /// 'Orig' instruction, identical in all ways except the
instruction
> /// has no parent, prev, or next.
> ///
> /// See also TargetInstrInfo::duplicate() for target-specific fixes to
cloned
> /// instructions.
which has since been changed to
> /// Create a new MachineInstr which is a copy of \p Orig, identical in all
> /// ways except the instruction has no parent, prev, or next. Bundling
flags
> /// are reset.
> ///
> /// Note: Clones a single instruction, not whole instruction bundles.
> /// Does not perform target specific adjustments; consider using
> /// TargetInstrInfo::duplicate() instead.
Based on either, I expected register ties to remain intact on the clone, 
but they are cleared. I can see how that happens (MachineInstr's 
constructor copies all machine operands one by one; when adding existing 
machine operands using MachineInstr::addOperand, ties are supposed to be 
cleared), but the documentation suggests to me the ties should be 
restored after all operands have been added.

Or is the new comment about bundling flags meant to cover register ties 
too? I think that's only about machine instruction bundles, but maybe 
I'm not understanding it right.

I can patch up register ties afterwards if necessary, but I'd like to 
know if this is a bug or not, because it affects whether I should do the 
patching up in my own pass or in the LLVM code, and whether I should 
report this as a bug.

Cheers,
Harald van Dijk

On 06/12/2018 16:11, Harald van Dijk via llvm-dev wrote:
> Hi,
> 
> Working on a new pass for a project based on LLVM 4.0.1 I tried to clone 
> a machine instruction using MachineFunction::CloneMachineInstr. Its 
> documentation reads
> 
>> /// CloneMachineInstr - Create a new MachineInstr which is a copy of
the
>> /// 'Orig' instruction, identical in all ways except the
instruction
>> /// has no parent, prev, or next.
>> ///
>> /// See also TargetInstrInfo::duplicate() for target-specific fixes to 
>> cloned
>> /// instructions.
> 
> which has since been changed to
> 
>> /// Create a new MachineInstr which is a copy of \p Orig, identical in 
>> all
>> /// ways except the instruction has no parent, prev, or next. Bundling 
>> flags
>> /// are reset.
>> ///
>> /// Note: Clones a single instruction, not whole instruction bundles.
>> /// Does not perform target specific adjustments; consider using
>> /// TargetInstrInfo::duplicate() instead.
> 
> Based on either, I expected register ties to remain intact on the clone, 
> but they are cleared. I can see how that happens (MachineInstr's 
> constructor copies all machine operands one by one; when adding existing 
> machine operands using MachineInstr::addOperand, ties are supposed to be 
> cleared), but the documentation suggests to me the ties should be 
> restored after all operands have been added.
> 
> Or is the new comment about bundling flags meant to cover register ties 
> too? I think that's only about machine instruction bundles, but maybe 
> I'm not understanding it right.
> 
> I can patch up register ties afterwards if necessary, but I'd like to 
> know if this is a bug or not, because it affects whether I should do the 
> patching up in my own pass or in the LLVM code, and whether I should 
> report this as a bug.
Because of the lack of response, I have reported it as a bug[1], but for 
now will treat CloneMachineInstr as if it is supposed to behave this 
way, and work around it in my own pass.

[1]: https://bugs.llvm.org/show_bug.cgi?id=40079
> Cheers,
> Harald van Dijk