[llvm] r224009 - Bugfix in InlineSpiller::traceSiblingValue().

[Quentin Colombet]

Hi David,

Indeed, this has been brought up during the review.

Jonas hasn’t been able to find a test case for in-tree targets so far.
I’ve given him a few hints that may help him to reproduce the problem with in-tree targets, but the conditions to make that happen are quite difficult to have all lined up (that’s why it never occurred so far, whereas the bug was there forever).

Jonas may have additional comments.

Thanks,
-Quentin

On Dec 11, 2014, at 9:58 AM, David Blaikie <dblaikie at gmail.com> wrote:

> Any chance of a test case? (I assume this was brought up in code review - but it's helpful to have the justification included in the commit message, ideally)
> 
> On Thu, Dec 11, 2014 at 2:40 AM, Patrik Hagglund <patrik.h.hagglund at ericsson.com> wrote:
> Author: patha
> Date: Thu Dec 11 04:40:17 2014
> New Revision: 224009
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=224009&view=rev
> Log:
> Bugfix in InlineSpiller::traceSiblingValue().
> 
> Properly determine whether or not a phi was added by splitting.
> Check against the current VNInfo of OrigLI instead of against the
> OrigVNI argument.
> 
> Patch provided by Jonas Paulsson. Reviewed by Quentin Colombet.
> 
> Modified:
>     llvm/trunk/lib/CodeGen/InlineSpiller.cpp
> 
> Modified: llvm/trunk/lib/CodeGen/InlineSpiller.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/InlineSpiller.cpp?rev=224009&r1=224008&r2=224009&view=diff
> ==============================================================================
> --- llvm/trunk/lib/CodeGen/InlineSpiller.cpp (original)
> +++ llvm/trunk/lib/CodeGen/InlineSpiller.cpp Thu Dec 11 04:40:17 2014
> @@ -508,6 +508,7 @@ MachineInstr *InlineSpiller::traceSiblin
>    SmallVector<std::pair<unsigned, VNInfo*>, 8> WorkList;
>    WorkList.push_back(std::make_pair(UseReg, UseVNI));
> 
> +  LiveInterval &OrigLI = LIS.getInterval(Original);
>    do {
>      unsigned Reg;
>      VNInfo *VNI;
> @@ -521,8 +522,11 @@ MachineInstr *InlineSpiller::traceSiblin
> 
>      // Trace through PHI-defs created by live range splitting.
>      if (VNI->isPHIDef()) {
> -      // Stop at original PHIs.  We don't know the value at the predecessors.
> -      if (VNI->def == OrigVNI->def) {
> +      // Stop at original PHIs.  We don't know the value at the
> +      // predecessors. Look up the VNInfo for the current definition
> +      // in OrigLI, to properly determine whether or not this phi was
> +      // added by splitting.
> +      if (VNI->def == OrigLI.getVNInfoAt(VNI->def)->def) {
>          DEBUG(dbgs() << "orig phi value\n");
>          SVI->second.DefByOrigPHI = true;
>          SVI->second.AllDefsAreReloads = false;
> @@ -542,7 +546,6 @@ MachineInstr *InlineSpiller::traceSiblin
>        // Separate all values dominated by OrigVNI into PHIs and non-PHIs.
>        SmallVector<VNInfo*, 8> PHIs, NonPHIs;
>        LiveInterval &LI = LIS.getInterval(Reg);
> -      LiveInterval &OrigLI = LIS.getInterval(Original);
> 
>        for (LiveInterval::vni_iterator VI = LI.vni_begin(), VE = LI.vni_end();
>             VI != VE; ++VI) {
> 
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits

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