<html><head><meta http-equiv="Content-Type" content="text/html charset=us-ascii"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;"><br><div><div>On Jul 12, 2013, at 4:20 AM, Chandler Carruth <<a href="mailto:chandlerc@google.com">chandlerc@google.com</a>> wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><div style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;"><div dir="ltr">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.</div></div></blockquote><div dir="auto"><br></div><div dir="auto">Many thanks for the test case. I cleaned up the code so corner cases should be more clear and reimplemented the patch in r186215.</div><div dir="auto">-Andy</div></div><div dir="auto"><br><blockquote type="cite"><div style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;"><div dir="ltr"><div><div>% cat bad2.ll </div><div>; ModuleID = '<stdin>'</div><div>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"</div><div>target triple = "x86_64-unknown-linux-gnu"</div><div><br></div><div>@.str = private unnamed_addr constant [4 x i8] c"%d\0A\00", align 1</div><div><br></div><div>; Function Attrs: nounwind uwtable</div><div>define i32 @main() #0 {</div><div>entry:</div><div> br label %do.body</div><div><br></div><div>do.body: ; preds = %do.body, %entry</div><div> %first.0 = phi i8 [ 0, %entry ], [ %inc, %do.body ]</div><div> %conv = zext i8 %first.0 to i32</div><div> %call = call i32 (i8*, ...)* @printf(i8* getelementptr inbounds ([4 x i8]* @.str, i64 0, i64 0), i32 %conv) #1</div><div> %inc = add i8 %first.0, 1</div><div> %cmp = icmp eq i8 %first.0, -1</div><div> br i1 %cmp, label %do.end, label %do.body</div><div><br></div><div>do.end: ; preds = %do.body</div><div> ret i32 0</div><div>}</div><div><br></div><div>declare i32 @printf(i8*, ...)</div><div><br></div><div>attributes #0 = { nounwind uwtable }</div><div>attributes #1 = { nounwind }</div></div><div><br></div><div>% ./bin/llc -O0 <bad2.ll | ./bin/clang -x assembler -<br></div><div>% ./a.out</div><div>0</div><div>1</div><div>2</div><div><snip></div><div>255</div><div><br></div><div>% ./bin/opt -S -indvars <bad2.ll | ./bin/llc -O0 | ./bin/clang -x assembler -<br></div><div>0</div><div>1</div><div>2</div><div><snip></div><div>255</div><div>256</div><div>257</div><div>...</div><div><br></div><div>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.</div><div><br></div><div>The bad2.ll test case is attached for easier access.</div><div><br></div></div><div class="gmail_extra"><br><br><div class="gmail_quote">On Thu, Jul 11, 2013 at 10:38 AM, Andrew Trick<span class="Apple-converted-space"> </span><span dir="ltr"><<a href="mailto:atrick@apple.com" target="_blank">atrick@apple.com</a>></span><span class="Apple-converted-space"> </span>wrote:<br><blockquote class="gmail_quote" style="margin: 0px 0px 0px 0.8ex; border-left-width: 1px; border-left-color: rgb(204, 204, 204); border-left-style: solid; padding-left: 1ex;"><div style="word-wrap: break-word;"><div><div class="im"><div>On Jul 11, 2013, at 12:29 AM, Michele Scandale <<a href="mailto:michele.scandale@gmail.com" target="_blank">michele.scandale@gmail.com</a>> wrote:</div><br><blockquote type="cite"><div style="font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px;"> Here an updated version of the patch fixing code style as suggested by Andrew.<br></div></blockquote><div dir="auto"><br></div></div>Committed r186107. Thanks.</div><div dir="auto">-Andy</div><div dir="auto"><br><div dir="auto"></div><blockquote type="cite"><div style="font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px;"><div><div class="h5"><br> Thanks again.<br><br> Michele Scandale<br><br>Hi atrick,<br><br><a href="http://llvm-reviews.chandlerc.com/D1112" target="_blank">http://llvm-reviews.chandlerc.com/D1112</a><br><br>CHANGE SINCE LAST DIFF<br> <a href="http://llvm-reviews.chandlerc.com/D1112?vs=2713&id=2765#toc" target="_blank">http://llvm-reviews.chandlerc.com/D1112?vs=2713&id=2765#toc</a><br><br>Files:<br> lib/Transforms/Scalar/IndVarSimplify.cpp<br> test/Transforms/IndVarSimplify/exitcnt-const-arstart-const-opt.ll<br><br>Index: lib/Transforms/Scalar/IndVarSimplify.cpp<br>===================================================================<br>--- lib/Transforms/Scalar/IndVarSimplify.cpp<br>+++ lib/Transforms/Scalar/IndVarSimplify.cpp<br>@@ -1612,10 +1612,29 @@<br> << " IVCount:\t" << *IVCount << "\n");<br><br> IRBuilder<> Builder(BI);<br>- if (SE->getTypeSizeInBits(CmpIndVar->getType())<br>- > SE->getTypeSizeInBits(ExitCnt->getType())) {<br>- CmpIndVar = Builder.CreateTrunc(CmpIndVar, ExitCnt->getType(),<br>- "lftr.wideiv");<br>+<br>+ unsigned CmpIndVarSize = SE->getTypeSizeInBits(CmpIndVar->getType());<br>+ unsigned ExitCntSize = SE->getTypeSizeInBits(ExitCnt->getType());<br>+ if (CmpIndVarSize > ExitCntSize) {<br>+ const SCEVAddRecExpr *AR = cast<SCEVAddRecExpr>(SE->getSCEV(IndVar));<br>+ const SCEV *ARStart = AR->getStart();<br>+ const SCEV *ARStep = AR->getStepRecurrence(*SE);<br>+ if (isa<SCEVConstant>(ARStart) && isa<SCEVConstant>(IVCount)) {<br>+ const APInt &Start = cast<SCEVConstant>(ARStart)->getValue()->getValue();<br>+ const APInt &Count = cast<SCEVConstant>(IVCount)->getValue()->getValue();<br>+<br>+ APInt NewLimit;<br>+ if (cast<SCEVConstant>(ARStep)->getValue()->isNegative())<br>+ NewLimit = Start - Count.zext(CmpIndVarSize);<br>+ else<br>+ NewLimit = Start + Count.zext(CmpIndVarSize);<br>+ ExitCnt = ConstantInt::get(CmpIndVar->getType(), NewLimit);<br>+<br>+ DEBUG(dbgs() << " Widen RHS:\t" << *ExitCnt << "\n");<br>+ } else {<br>+ CmpIndVar = Builder.CreateTrunc(CmpIndVar, ExitCnt->getType(),<br>+ "lftr.wideiv");<br>+ }<br> }<br><br> Value *Cond = Builder.CreateICmp(P, CmpIndVar, ExitCnt, "exitcond");<br>Index: test/Transforms/IndVarSimplify/exitcnt-const-arstart-const-opt.ll<br>===================================================================<br>--- test/Transforms/IndVarSimplify/exitcnt-const-arstart-const-opt.ll<br>+++ test/Transforms/IndVarSimplify/exitcnt-const-arstart-const-opt.ll<br>@@ -0,0 +1,25 @@<br>+;RUN: opt -S %s -indvars | FileCheck %s<br>+<br>+; Function Attrs: nounwind uwtable<br>+define void @foo() #0 {<br>+entry:<br>+ br label %for.body<br>+<br>+for.body: ; preds = %entry, %for.body<br>+ %i.01 = phi i16 [ 0, %entry ], [ %inc, %for.body ]<br>+ %conv2 = sext i16 %i.01 to i32<br>+ call void @bar(i32 %conv2) #1<br>+ %inc = add i16 %i.01, 1<br>+;CHECK-NOT: %lftr.wideiv = trunc i32 %indvars.iv.next to i16<br>+;CHECK: %exitcond = icmp ne i32 %indvars.iv.next, 512<br>+ %cmp = icmp slt i16 %inc, 512<br>+ br i1 %cmp, label %for.body, label %for.end<br>+<br>+for.end: ; preds = %for.body<br>+ ret void<br>+}<br>+<br>+declare void @bar(i32)<br>+<br>+attributes #0 = { nounwind uwtable }<br>+attributes #1 = { nounwind }<br></div></div><span><D1112.2.patch></span>_______________________________________________<br>llvm-commits mailing list<br><a href="mailto:llvm-commits@cs.uiuc.edu" target="_blank">llvm-commits@cs.uiuc.edu</a><br><a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a></div></blockquote></div><br></div><br>_______________________________________________<br>llvm-commits mailing list<br><a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br><a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br><br></blockquote></div><br></div><span><bad2.ll></span></div></blockquote></div><br></body></html>