[PATCH] [MachineSinking] Conservatively clear kill flags after coalescing.

[Quentin Colombet]

Hi Patrik,

LGTM.

Thanks,
-Quentin
On Sep 5, 2014, at 1:03 AM, Patrik Hägglund H <patrik.h.hagglund at ericsson.com> wrote:

> Hi Quentin,
>  
> Jonas looked further into the problem below, and asked me to submit his patch. Note the we have our own out-of-tree target, and we have not been able to reproduce this problem on an in-tree target. /Patrik Hägglund
>  
> [MachineSinking] Conservatively clear kill flags after coalescing.
>  
> This solves the problem of having a kill flag inside a loop
> with a definition of the register prior to the loop:
>  
> %vreg368<def> ...
>  
> Inside loop:
>  
>         %vreg520<def> = COPY %vreg368
>         %vreg568<def,tied1> = add %vreg341<tied0>, %vreg520<kill>
>  
> => was coalesced into =>
>  
>         %vreg568<def,tied1> = add %vreg341<tied0>, %vreg368<kill>
>  
> MachineVerifier then complained:
> *** Bad machine code: Virtual register killed in block, but needed live out. ***
>  
> The kill flag for %vreg368 is incorrect, and is cleared by this patch.
>  
> This is similar to the clearing done at the end of
> MachineSinking::SinkInstruction().
> ---
> lib/CodeGen/MachineSink.cpp | 5 +++++
> 1 file changed, 5 insertions(+)
>  
> diff --git a/lib/CodeGen/MachineSink.cpp b/lib/CodeGen/MachineSink.cpp
> index 7782001..261af54 100644
> --- a/lib/CodeGen/MachineSink.cpp
> +++ b/lib/CodeGen/MachineSink.cpp
> @@ -157,6 +157,11 @@ bool MachineSinking::PerformTrivialForwardCoalescing(MachineInstr *MI,
>    DEBUG(dbgs() << "*** to: " << *MI);
>    MRI->replaceRegWith(DstReg, SrcReg);
>    MI->eraseFromParent();
> +
> +  // Conservatively, clear any kill flags, since it's possible that they are no
> +  // longer correct.
> +  MRI->clearKillFlags(SrcReg);
> +
>    ++NumCoalesces;
>    return true;
> }
> --
> 2.1.0
>  
> From: llvmdev-bounces at cs.uiuc.edu [mailto:llvmdev-bounces at cs.uiuc.edu] On Behalf Of Quentin Colombet
> Sent: den 2 september 2014 18:37
> To: Jonas Paulsson
> Cc: llvmdev at cs.uiuc.edu
> Subject: Re: [LLVMdev] Machine code sinking pass
>  
> Hi Jonas,
>  
> This looks like a bug in Machine code sinking pass.
>  
> Please file a PR on llvm.org/bugs with a reduced test case to keep track of the issue.
>  
> Thanks,
> -Quentin
>  
> On Sep 2, 2014, at 5:57 AM, Jonas Paulsson <jonas.paulsson at ericsson.com> wrote:
> 
> 
> Hi,
>  
> I ran into MachineVerifier ”Virtual register killed in block, but needed live out.”
>  
> It was MachineSinking:PerformTrivialForwardCoalescing() that coalesced a COPY inside a single-block loop, but left the
> kill-flag and then MachineVerifier complains that a register in vregsRequired is killed in MBB.
>  
> In the example, %vreg520 is replaced by %vreg368 by PerformTrivialForwardCoalescing(), without clearing the kill flag:
>  
> BB#13: derived from LLVM BB %CF250
>     Predecessors according to CFG: BB#12 BB#13 BB#14
>        …
>         %vreg520<def> = COPY %vreg368
>         %vreg568<def,tied1> = cmp %vreg341<tied0>, %vreg520<kill>
>         brr_cond <BB#13>
>         brr_uncond <BB#14>
>     Successors according to CFG: BB#13, BB#14
>  
> Into
>  
> BB#13: derived from LLVM BB %CF250
>     Predecessors according to CFG: BB#12 BB#13 BB#14
>         …
>         %vreg568<def,tied1> = cmp %vreg341<tied0>, %vreg368<kill>
>         brr_cond <BB#13>
>         brr_uncond <BB#14>
>  
> =>
>  
> *** Bad machine code: Virtual register killed in block, but needed live out. ***
> - function:    autogen_SD15028
> - basic block: BB#13 CF250 (0x1c75890)
> Virtual register %vreg368 is used after the block.
>  
> There is only one use of %vreg368 in the function.
>  
> One thing that strikes me is that one might want to clear the kill flag for a use inside a loop of a register defined prior to
> the loop?
>  
> Best regards,
>  
> Jonas Paulsson
>  
> _______________________________________________
> LLVM Developers mailing list
> LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
>  
> <0001-MachineSinking-Conservatively-clear-kill-flags-after.patch>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140905/80ba9534/attachment.html>