[PATCH] Little improvement in IndVarSimplify

[Andrew Trick]

On Jul 12, 2013, at 4:20 AM, Chandler Carruth <chandlerc at google.com> wrote:

> I've reverted in r186152. It caused a really serious regression. Test case attached here (it's not quite in the right form for committing yet -- you can probably write it more directly against indvars once debugged.

Many thanks for the test case. I cleaned up the code so corner cases should be more clear and reimplemented the patch in r186215.
-Andy

> % cat bad2.ll                                        
> ; ModuleID = '<stdin>'
> target datalayout = "e-p:64:64:64-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:64:64-f32:32:32-f64:64:64-v64:64:64-v128:128:128-a0:0:64-s0:64:64-f80:128:128-n8:16:32:64-S128"
> target triple = "x86_64-unknown-linux-gnu"
> 
> @.str = private unnamed_addr constant [4 x i8] c"%d\0A\00", align 1
> 
> ; Function Attrs: nounwind uwtable
> define i32 @main() #0 {
> entry:
>   br label %do.body
> 
> do.body:                                          ; preds = %do.body, %entry
>   %first.0 = phi i8 [ 0, %entry ], [ %inc, %do.body ]
>   %conv = zext i8 %first.0 to i32
>   %call = call i32 (i8*, ...)* @printf(i8* getelementptr inbounds ([4 x i8]* @.str, i64 0, i64 0), i32 %conv) #1
>   %inc = add i8 %first.0, 1
>   %cmp = icmp eq i8 %first.0, -1
>   br i1 %cmp, label %do.end, label %do.body
> 
> do.end:                                           ; preds = %do.body
>   ret i32 0
> }
> 
> declare i32 @printf(i8*, ...)
> 
> attributes #0 = { nounwind uwtable }
> attributes #1 = { nounwind }
> 
> % ./bin/llc -O0 <bad2.ll | ./bin/clang -x assembler -
> % ./a.out
> 0
> 1
> 2
> <snip>
> 255
> 
> % ./bin/opt -S -indvars <bad2.ll | ./bin/llc -O0 | ./bin/clang -x assembler -
> 0
> 1
> 2
> <snip>
> 255
> 256
> 257
> ...
> 
> This goes on until the 64-bit register wraps back to zero. ;] It looks like the problem is extending the integer induction variable and exit value without accounting for wrapping.
> 
> The bad2.ll test case is attached for easier access.
> 
> 
> 
> On Thu, Jul 11, 2013 at 10:38 AM, Andrew Trick <atrick at apple.com> wrote:
> On Jul 11, 2013, at 12:29 AM, Michele Scandale <michele.scandale at gmail.com> wrote:
> 
>>  Here an updated version of the patch fixing code style as suggested by Andrew.
> 
> Committed r186107. Thanks.
> -Andy
> 
>> 
>>  Thanks again.
>> 
>>  Michele Scandale
>> 
>> Hi atrick,
>> 
>> http://llvm-reviews.chandlerc.com/D1112
>> 
>> CHANGE SINCE LAST DIFF
>>  http://llvm-reviews.chandlerc.com/D1112?vs=2713&id=2765#toc
>> 
>> Files:
>>  lib/Transforms/Scalar/IndVarSimplify.cpp
>>  test/Transforms/IndVarSimplify/exitcnt-const-arstart-const-opt.ll
>> 
>> Index: lib/Transforms/Scalar/IndVarSimplify.cpp
>> ===================================================================
>> --- lib/Transforms/Scalar/IndVarSimplify.cpp
>> +++ lib/Transforms/Scalar/IndVarSimplify.cpp
>> @@ -1612,10 +1612,29 @@
>>                << "  IVCount:\t" << *IVCount << "\n");
>> 
>>   IRBuilder<> Builder(BI);
>> -  if (SE->getTypeSizeInBits(CmpIndVar->getType())
>> -      > SE->getTypeSizeInBits(ExitCnt->getType())) {
>> -    CmpIndVar = Builder.CreateTrunc(CmpIndVar, ExitCnt->getType(),
>> -                                    "lftr.wideiv");
>> +
>> +  unsigned CmpIndVarSize = SE->getTypeSizeInBits(CmpIndVar->getType());
>> +  unsigned ExitCntSize = SE->getTypeSizeInBits(ExitCnt->getType());
>> +  if (CmpIndVarSize > ExitCntSize) {
>> +    const SCEVAddRecExpr *AR = cast<SCEVAddRecExpr>(SE->getSCEV(IndVar));
>> +    const SCEV *ARStart = AR->getStart();
>> +    const SCEV *ARStep = AR->getStepRecurrence(*SE);
>> +    if (isa<SCEVConstant>(ARStart) && isa<SCEVConstant>(IVCount)) {
>> +      const APInt &Start = cast<SCEVConstant>(ARStart)->getValue()->getValue();
>> +      const APInt &Count = cast<SCEVConstant>(IVCount)->getValue()->getValue();
>> +
>> +      APInt NewLimit;
>> +      if (cast<SCEVConstant>(ARStep)->getValue()->isNegative())
>> +        NewLimit = Start - Count.zext(CmpIndVarSize);
>> +      else
>> +        NewLimit = Start + Count.zext(CmpIndVarSize);
>> +      ExitCnt = ConstantInt::get(CmpIndVar->getType(), NewLimit);
>> +
>> +      DEBUG(dbgs() << "  Widen RHS:\t" << *ExitCnt << "\n");
>> +    } else {
>> +      CmpIndVar = Builder.CreateTrunc(CmpIndVar, ExitCnt->getType(),
>> +                                      "lftr.wideiv");
>> +    }
>>   }
>> 
>>   Value *Cond = Builder.CreateICmp(P, CmpIndVar, ExitCnt, "exitcond");
>> Index: test/Transforms/IndVarSimplify/exitcnt-const-arstart-const-opt.ll
>> ===================================================================
>> --- test/Transforms/IndVarSimplify/exitcnt-const-arstart-const-opt.ll
>> +++ test/Transforms/IndVarSimplify/exitcnt-const-arstart-const-opt.ll
>> @@ -0,0 +1,25 @@
>> +;RUN: opt -S %s -indvars | FileCheck %s
>> +
>> +; Function Attrs: nounwind uwtable
>> +define void @foo() #0 {
>> +entry:
>> +  br label %for.body
>> +
>> +for.body:                                         ; preds = %entry, %for.body
>> +  %i.01 = phi i16 [ 0, %entry ], [ %inc, %for.body ]
>> +  %conv2 = sext i16 %i.01 to i32
>> +  call void @bar(i32 %conv2) #1
>> +  %inc = add i16 %i.01, 1
>> +;CHECK-NOT: %lftr.wideiv = trunc i32 %indvars.iv.next to i16
>> +;CHECK: %exitcond = icmp ne i32 %indvars.iv.next, 512
>> +  %cmp = icmp slt i16 %inc, 512
>> +  br i1 %cmp, label %for.body, label %for.end
>> +
>> +for.end:                                          ; preds = %for.body
>> +  ret void
>> +}
>> +
>> +declare void @bar(i32)
>> +
>> +attributes #0 = { nounwind uwtable }
>> +attributes #1 = { nounwind }
>> <D1112.2.patch>_______________________________________________
>> 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
> 
> 
> <bad2.ll>

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