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

[Mikael Holmén via llvm-commits]

Hi Wei,

On 07/05/2016 08:05 PM, Wei Mi wrote:
> On Mon, Jul 4, 2016 at 4:52 AM, Mikael Holmén
> <mikael.holmen at ericsson.com> wrote:
>> Hi Wei,
>>
>> On 05/17/2016 11:16 PM, Quentin Colombet wrote:
>>>
>>>
>>>> On May 12, 2016, at 5:40 AM, Mikael Holmén <mikael.holmen at ericsson.com>
>>>> wrote:
>>>>
>>>> Hi Wei and Quentin,
>>>>
>>>> On 05/10/2016 07:55 PM, Wei Mi wrote:
>>>> [...]
>>>>>
>>>>> Mikael, thanks for getting the dump. The problem is that in Phoenix
>>>>> Post Coalescer, the following instruction becomes dead and it is
>>>>> marked as dead, but it is never deleted until regsplit in regalloc
>>>>> finds it dead and calls eliminateDeadDef for it.
>>>>>
>>>>> %vreg51<def,dead,tied1> = addh_nimm5_a32_aNhDst_formatDstN32
>>>>> %vreg51<tied0>, 4, pred:0, pred:%noreg, pred:0, %CCReg<imp-def,dead>,
>>>>> %cuc<imp-use>; aNh_0_7:%vreg51
>>>>
>>>>
>>>> Yes, this instruction is introduced by our software pipeline pass running
>>>> post coalescer. In the epilogue produced by the software pipeliner there may
>>>> be dead code.
>>>>
>>>>> Maybe regalloc should allow IR containing dead instruction as its
>>>>> input, since itself leaves some dead instructions in the IR for the
>>>>> sake of better rematerialization.  Quentin, what do you think? If that
>>>>> is the case, the patch D19486 maybe needed.
>>>
>>>
>>> As long as the IR is valid then, yes, we should add the required bits to
>>> support it.
>>
>>
>> Did you get anywhere with this?
>>
>> As I think I said in some earlier mail, we've worked around the last problem
>> that we're aware of with
>>
>> +  // 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
>> +  if (MI->isRegTiedToUseOperand(0)) {
>> +    DEBUG(dbgs() << "Can't delete, def is tied to operand: " << Idx << '\t'
>> << *MI);
>> +    return;
>> +  }
>> +
>>
>> in LiveRangeEdit::eliminateDeadDef but it sounded like you had some better
>> solution, D19486?
>>
>> Thanks,
>> Mikael
>>
>
> Hi Mikael,
>
> As discussed with Quentin, D19486 is only a workaround. I havn't got
> time to solve the essential problem there yet, but the job is my todo
> list.

Ok, sounds good!

> Is it helpful for your case to checkin D19486? If it is true, I
> can checkin D19486 first and solve the live-range update problem in
> the future.

Sure, if it has no downsides then please do. Even if just a workaround 
it solves the problem we saw so if it's ok with everyone then please 
push it.

Thanks,
Mikael

>
> Thanks,
> Wei.
>