[llvm] r222040 - InstCombine: Fix infinite loop caused by visitFPTrunc

[Manman Ren]

Hi David,

The bot is still failing after reverting r222040. What I did was:
with r222059, I built a selfhost lto of that revision, and got the same error.
with r222040, I didn’t do a make clean, built another selfhost lto to the same build directory, and it still failed
with r222038, still no make clean, built another selfhost lto to the same build directory, and it succeeded
with r222039, still no make clean, built another selfhost lto to the same build directory, and it succeeded

The only step that is different from public bot is I didn’t start with a new build directory.
You can re-submit this patch now. I will try to investigate with a new build directory for each revision.

Thanks and sorry for the noise,

Manman

> On Nov 17, 2014, at 4:22 PM, David Majnemer <david.majnemer at gmail.com> wrote:
> 
> Sure, feel free to revert.
> 
> On Mon, Nov 17, 2014 at 4:17 PM, Manman Ren <mren at apple.com> wrote:
> 
> Hi David,
> 
> I think this breaks the public stage2 builedbot: http://lab.llvm.org:8080/green/job/clang-Rlto_master/298/
> 
> r222039 builds fine, but r222040 does not, from my local debugging.
> 
> Can we revert this commit and see if the bot becomes green?
> 
> Thanks,
> Manman
> 
> > On Nov 14, 2014, at 1:21 PM, David Majnemer <david.majnemer at gmail.com> wrote:
> >
> > Author: majnemer
> > Date: Fri Nov 14 15:21:15 2014
> > New Revision: 222040
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=222040&view=rev
> > Log:
> > InstCombine: Fix infinite loop caused by visitFPTrunc
> >
> > We would attempt to replace a fptrunc of an frem with an identical
> > fptrunc.  This would cause the new fptrunc to be added to the worklist.
> > Of course, this results in an infinite loop because we will keep
> > visiting the newly created fptruncs.
> >
> > This fixes PR21576.
> >
> > Modified:
> >    llvm/trunk/lib/Transforms/InstCombine/InstCombineCasts.cpp
> >    llvm/trunk/test/Transforms/InstCombine/fpcast.ll
> >
> > Modified: llvm/trunk/lib/Transforms/InstCombine/InstCombineCasts.cpp
> > URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/InstCombine/InstCombineCasts.cpp?rev=222040&r1=222039&r2=222040&view=diff
> > ==============================================================================
> > --- llvm/trunk/lib/Transforms/InstCombine/InstCombineCasts.cpp (original)
> > +++ llvm/trunk/lib/Transforms/InstCombine/InstCombineCasts.cpp Fri Nov 14 15:21:15 2014
> > @@ -1269,14 +1269,17 @@ Instruction *InstCombiner::visitFPTrunc(
> >         // type of OpI doesn't enter into things at all.  We simply evaluate
> >         // in whichever source type is larger, then convert to the
> >         // destination type.
> > +        Value *NewLHS = LHSOrig, *NewRHS = RHSOrig;
> >         if (LHSWidth < SrcWidth)
> > -          LHSOrig = Builder->CreateFPExt(LHSOrig, RHSOrig->getType());
> > +          NewLHS = Builder->CreateFPExt(NewLHS, RHSOrig->getType());
> >         else if (RHSWidth <= SrcWidth)
> > -          RHSOrig = Builder->CreateFPExt(RHSOrig, LHSOrig->getType());
> > -        Value *ExactResult = Builder->CreateFRem(LHSOrig, RHSOrig);
> > -        if (Instruction *RI = dyn_cast<Instruction>(ExactResult))
> > -          RI->copyFastMathFlags(OpI);
> > -        return CastInst::CreateFPCast(ExactResult, CI.getType());
> > +          NewRHS = Builder->CreateFPExt(NewRHS, LHSOrig->getType());
> > +        if (NewLHS != LHSOrig || NewRHS != RHSOrig) {
> > +          Value *ExactResult = Builder->CreateFRem(NewLHS, NewRHS);
> > +          if (Instruction *RI = dyn_cast<Instruction>(ExactResult))
> > +            RI->copyFastMathFlags(OpI);
> > +          return CastInst::CreateFPCast(ExactResult, CI.getType());
> > +        }
> >     }
> >
> >     // (fptrunc (fneg x)) -> (fneg (fptrunc x))
> >
> > Modified: llvm/trunk/test/Transforms/InstCombine/fpcast.ll
> > URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/InstCombine/fpcast.ll?rev=222040&r1=222039&r2=222040&view=diff
> > ==============================================================================
> > --- llvm/trunk/test/Transforms/InstCombine/fpcast.ll (original)
> > +++ llvm/trunk/test/Transforms/InstCombine/fpcast.ll Fri Nov 14 15:21:15 2014
> > @@ -53,3 +53,13 @@ define half @test5(float %a, float %b, f
> > }
> >
> > declare float @llvm.fabs.f32(float) nounwind readonly
> > +
> > +define <1 x float> @test6(<1 x double> %V) {
> > +  %frem = frem <1 x double> %V, %V
> > +  %trunc = fptrunc <1 x double> %frem to <1 x float>
> > +  ret <1 x float> %trunc
> > +; CHECK-LABEL: @test6
> > +; CHECK-NEXT: %[[frem:.*]]  = frem <1 x double> %V, %V
> > +; CHECK-NEXT: %[[trunc:.*]] = fptrunc <1 x double> %[[frem]] to <1 x float>
> > +; CHECK-NEXT: ret <1 x float> %trunc
> > +}
> >
> >
> > _______________________________________________
> > 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/20141118/4e9a34cf/attachment.html>