Another problem with "Recommit r265547, and r265610,r265639,r265657"

Mikael Holmén via llvm-commits llvm-commits at lists.llvm.org
Mon May 9 00:22:47 PDT 2016


Hi Wei,

(Sorry for not getting back earlier, but I've been ill.)

On 05/02/2016 06:31 PM, Wei Mi wrote:
[...]
>> So, this fix in RegisterCoalescer solves the last problem I ran into.
>>
>> But the previous problem where we had an instruction with a tied operand...
>> How do we solve that?
>>
>> We've done
>>
>> @@ -257,0 +258,7 @@ void LiveRangeEdit::eliminateDeadDef(MachineInstr *MI,
>> ToShrinkSet &ToShrink) {
>> +  // To avoid situations where Def is tied and the same Def/Use registers
>> gets
>> +  //  new reg ids and we end up with a use of an undefined register (bug
>> #10189)
>> +  if (MI->isRegTiedToUseOperand(0)) {
>> +    DEBUG(dbgs() << "Can't delete, def is tied to operand: " << Idx << '\t'
>> << *MI);
>> +    return;
>> +  }
>> +
>>
>> early in LiveRangeEdit::eliminateDeadDef but I'm not sure if you had a
>> better fix for it?
>>
>
> Instruction with a tied operand is not triviallyRematerializable so
> the problem shouldn't appear either. If that problem still exists with
> your patch in RegisterCoalescer, we need to look deeply into what is
> wrong there to come up a better fix.

The problem still exists if only using my fix in RegisterCoalescer.

However, if applying your patch http://reviews.llvm.org/D19486 it goes 
away. Do you intend to push that?

For a while I thought your patch wouldn't be necessary with the 
RegisterCoalescer change, but maybe it is for the tied-operand case then?

Thanks,
Mikael



More information about the llvm-commits mailing list