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

[Wei Mi via llvm-commits]

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. 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.

Thanks,
Wei.