[llvm] r224097 - Fix another infinite loop in InstCombine

Steven Wu stevenwu at apple.com
Fri Dec 12 09:23:04 PST 2014


Committed in r224133.

Steven

> On Dec 12, 2014, at 9:01 AM, Bob Wilson <bob.wilson at apple.com> wrote:
> 
> One minor stylistic comment below.
> 
>> On Dec 11, 2014, at 8:34 PM, Steven Wu <stevenwu at apple.com> wrote:
>> 
>> Author: steven_wu
>> Date: Thu Dec 11 22:34:07 2014
>> New Revision: 224097
>> 
>> URL: http://llvm.org/viewvc/llvm-project?rev=224097&view=rev
>> Log:
>> Fix another infinite loop in InstCombine
>> 
>> Summary:
>> InstCombine infinite-loops for the testcase added
>> It is because InstCombine is generating instructions that can be
>> optimized by itself. Fix by not optimizing frem if the optimized
>> type is the same as original type.
>> rdar://problem/19150820
>> 
>> Reviewers: majnemer
>> 
>> Differential Revision: http://reviews.llvm.org/D6634
>> 
>> 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=224097&r1=224096&r2=224097&view=diff
>> ==============================================================================
>> --- llvm/trunk/lib/Transforms/InstCombine/InstCombineCasts.cpp (original)
>> +++ llvm/trunk/lib/Transforms/InstCombine/InstCombineCasts.cpp Thu Dec 11 22:34:07 2014
>> @@ -1269,16 +1269,19 @@ 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.
>> -        if (LHSWidth < SrcWidth)
>> -          LHSOrig = Builder->CreateFPExt(LHSOrig, RHSOrig->getType());
>> -        else if (RHSWidth <= SrcWidth)
>> -          RHSOrig = Builder->CreateFPExt(RHSOrig, LHSOrig->getType());
>> -        if (LHSOrig != OpI->getOperand(0) || RHSOrig != OpI->getOperand(1)) {
>> -          Value *ExactResult = Builder->CreateFRem(LHSOrig, RHSOrig);
>> -          if (Instruction *RI = dyn_cast<Instruction>(ExactResult))
>> -            RI->copyFastMathFlags(OpI);
>> -          return CastInst::CreateFPCast(ExactResult, CI.getType());
>> +        if (SrcWidth != OpWidth) {
> 
> Can you flip this comparison and break out of the case when the widths are equal? Then you don’t need to increase the indentation of the other code.
> 
>> +          if (LHSWidth < SrcWidth)
>> +            LHSOrig = Builder->CreateFPExt(LHSOrig, RHSOrig->getType());
>> +          else if (RHSWidth <= SrcWidth)
>> +            RHSOrig = Builder->CreateFPExt(RHSOrig, LHSOrig->getType());
>> +          if (LHSOrig != OpI->getOperand(0) || RHSOrig != OpI->getOperand(1)) {
>> +            Value *ExactResult = Builder->CreateFRem(LHSOrig, RHSOrig);
>> +            if (Instruction *RI = dyn_cast<Instruction>(ExactResult))
>> +              RI->copyFastMathFlags(OpI);
>> +            return CastInst::CreateFPCast(ExactResult, CI.getType());
>> +          }
>>        }
>> +        break;
>>    }
>> 
>>    // (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=224097&r1=224096&r2=224097&view=diff
>> ==============================================================================
>> --- llvm/trunk/test/Transforms/InstCombine/fpcast.ll (original)
>> +++ llvm/trunk/test/Transforms/InstCombine/fpcast.ll Thu Dec 11 22:34:07 2014
>> @@ -73,3 +73,15 @@ define float @test7(double %V) {
>> ; CHECK-NEXT: %[[trunc:.*]] = fptrunc double %frem to float
>> ; CHECK-NEXT: ret float %trunc
>> }
>> +
>> +define float @test8(float %V) {
>> +  %fext = fpext float %V to double
>> +  %frem = frem double %fext, 1.000000e-01
>> +  %trunc = fptrunc double %frem to float
>> +  ret float %trunc
>> +; CHECK-LABEL: @test8
>> +; CHECK-NEXT: %[[fext:.*]]  = fpext float %V to double
>> +; CHECK-NEXT: %[[frem:.*]]  = frem double %fext, 1.000000e-01
>> +; CHECK-NEXT: %[[trunc:.*]] = fptrunc double %frem to float
>> +; CHECK-NEXT: ret float %trunc
>> +}
>> 
>> 
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
> 





More information about the llvm-commits mailing list