[llvm] r230280 - Bugfix: SCEVExpander incorrectly marks increment operations as no-wrap

Sanjoy Das sanjoy at playingwithpointers.com
Tue Feb 24 13:57:54 PST 2015


Thanks for taking care of this, and my apologies for not responding
quickly.  I'll re-commit a fixed version soon.

-- Sanjoy

On Tue, Feb 24, 2015 at 8:22 AM, Hans Wennborg <hans at chromium.org> wrote:
> I've reverted this in r230341 to unbreak our builds until this can be
> fixed properly.
>
>  - Hans
>
> On Tue, Feb 24, 2015 at 5:52 AM, H.J. Lu <hjl.tools at gmail.com> wrote:
>> It caused:
>>
>> http://llvm.org/bugs/show_bug.cgi?id=22674
>>
>> On Mon, Feb 23, 2015 at 3:23 PM, Sanjoy Das
>> <sanjoy at playingwithpointers.com> wrote:
>>> Author: sanjoy
>>> Date: Mon Feb 23 17:22:58 2015
>>> New Revision: 230280
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=230280&view=rev
>>> Log:
>>> Bugfix: SCEVExpander incorrectly marks increment operations as no-wrap
>>>
>>> When emitting the increment operation, SCEVExpander marks the
>>> operation as nuw or nsw based on the flags on the preincrement SCEV.
>>> This is incorrect because, for instance, it is possible that {-6,+,1}
>>> is <nuw> while {-6,+,1}+1 = {-5,+,1} is not.
>>>
>>> This change teaches SCEV to mark the increment as nuw/nsw only if it
>>> can explicitly prove that the increment operation won't overflow.
>>>
>>> Apart from the attached test case, another (more realistic) manifestation
>>> of the bug can be seen in Transforms/IndVarSimplify/pr20680.ll.
>>>
>>> NOTE: this change was landed with an incorrect commit message in
>>> rL230275 and was reverted for that reason in rL230279.  This commit
>>> message is the correct one.
>>>
>>> Differential Revision: http://reviews.llvm.org/D7778
>>>
>>>
>>> Added:
>>>     llvm/trunk/test/Analysis/ScalarEvolution/scev-expander-incorrect-nowrap.ll
>>> Modified:
>>>     llvm/trunk/lib/Analysis/ScalarEvolutionExpander.cpp
>>>     llvm/trunk/test/Analysis/ScalarEvolution/zext-signed-addrec.ll
>>>     llvm/trunk/test/CodeGen/AArch64/arm64-scaled_iv.ll
>>>     llvm/trunk/test/CodeGen/X86/avoid_complex_am.ll
>>>     llvm/trunk/test/Transforms/IndVarSimplify/overflowcheck.ll
>>>     llvm/trunk/test/Transforms/IndVarSimplify/pr20680.ll
>>>     llvm/trunk/test/Transforms/LoopStrengthReduce/count-to-zero.ll
>>>     llvm/trunk/test/Transforms/LoopStrengthReduce/uglygep.ll
>>>
>>> Modified: llvm/trunk/lib/Analysis/ScalarEvolutionExpander.cpp
>>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Analysis/ScalarEvolutionExpander.cpp?rev=230280&r1=230279&r2=230280&view=diff
>>> ==============================================================================
>>> --- llvm/trunk/lib/Analysis/ScalarEvolutionExpander.cpp (original)
>>> +++ llvm/trunk/lib/Analysis/ScalarEvolutionExpander.cpp Mon Feb 23 17:22:58 2015
>>> @@ -1063,6 +1063,34 @@ static bool canBeCheaplyTransformed(Scal
>>>    return false;
>>>  }
>>>
>>> +static bool IsIncrementNSW(ScalarEvolution &SE, const SCEVAddRecExpr *AR) {
>>> +  if (!isa<IntegerType>(AR->getType()))
>>> +    return false;
>>> +
>>> +  unsigned BitWidth = cast<IntegerType>(AR->getType())->getBitWidth();
>>> +  Type *WideTy = IntegerType::get(AR->getType()->getContext(), BitWidth * 2);
>>> +  const SCEV *Step = AR->getStepRecurrence(SE);
>>> +  const SCEV *OpAfterExtend = SE.getAddExpr(SE.getSignExtendExpr(Step, WideTy),
>>> +                                            SE.getSignExtendExpr(AR, WideTy));
>>> +  const SCEV *ExtendAfterOp =
>>> +    SE.getSignExtendExpr(SE.getAddExpr(AR, Step), WideTy);
>>> +  return ExtendAfterOp == OpAfterExtend;
>>> +}
>>> +
>>> +static bool IsIncrementNUW(ScalarEvolution &SE, const SCEVAddRecExpr *AR) {
>>> +  if (!isa<IntegerType>(AR->getType()))
>>> +    return false;
>>> +
>>> +  unsigned BitWidth = cast<IntegerType>(AR->getType())->getBitWidth();
>>> +  Type *WideTy = IntegerType::get(AR->getType()->getContext(), BitWidth * 2);
>>> +  const SCEV *Step = AR->getStepRecurrence(SE);
>>> +  const SCEV *OpAfterExtend = SE.getAddExpr(SE.getZeroExtendExpr(Step, WideTy),
>>> +                                            SE.getZeroExtendExpr(AR, WideTy));
>>> +  const SCEV *ExtendAfterOp =
>>> +    SE.getZeroExtendExpr(SE.getAddExpr(AR, Step), WideTy);
>>> +  return ExtendAfterOp == OpAfterExtend;
>>> +}
>>> +
>>>  /// getAddRecExprPHILiterally - Helper for expandAddRecExprLiterally. Expand
>>>  /// the base addrec, which is the addrec without any non-loop-dominating
>>>  /// values, and return the PHI.
>>> @@ -1213,10 +1241,11 @@ SCEVExpander::getAddRecExprPHILiterally(
>>>        IVIncInsertPos : Pred->getTerminator();
>>>      Builder.SetInsertPoint(InsertPos);
>>>      Value *IncV = expandIVInc(PN, StepV, L, ExpandTy, IntTy, useSubtract);
>>> +
>>>      if (isa<OverflowingBinaryOperator>(IncV)) {
>>> -      if (Normalized->getNoWrapFlags(SCEV::FlagNUW))
>>> +      if (IsIncrementNUW(SE, Normalized))
>>>          cast<BinaryOperator>(IncV)->setHasNoUnsignedWrap();
>>> -      if (Normalized->getNoWrapFlags(SCEV::FlagNSW))
>>> +      if (IsIncrementNSW(SE, Normalized))
>>>          cast<BinaryOperator>(IncV)->setHasNoSignedWrap();
>>>      }
>>>      PN->addIncoming(IncV, Pred);
>>>
>>> Added: llvm/trunk/test/Analysis/ScalarEvolution/scev-expander-incorrect-nowrap.ll
>>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Analysis/ScalarEvolution/scev-expander-incorrect-nowrap.ll?rev=230280&view=auto
>>> ==============================================================================
>>> --- llvm/trunk/test/Analysis/ScalarEvolution/scev-expander-incorrect-nowrap.ll (added)
>>> +++ llvm/trunk/test/Analysis/ScalarEvolution/scev-expander-incorrect-nowrap.ll Mon Feb 23 17:22:58 2015
>>> @@ -0,0 +1,30 @@
>>> +; RUN: opt -indvars -S < %s | FileCheck %s
>>> +
>>> +declare void @use(i32)
>>> +declare void @use.i8(i8)
>>> +
>>> +define void @f() {
>>> +; CHECK-LABEL: @f
>>> + entry:
>>> +  br label %loop
>>> +
>>> + loop:
>>> +; The only use for idx.mirror is to induce an nuw for %idx.  It does
>>> +; not induce an nuw for %idx.inc
>>> +  %idx.mirror = phi i8 [ -6, %entry ], [ %idx.mirror.inc, %loop ]
>>> +  %idx = phi i8 [ -5, %entry ], [ %idx.inc, %loop ]
>>> +
>>> +  %idx.sext = sext i8 %idx to i32
>>> +  call void @use(i32 %idx.sext)
>>> +
>>> +  %idx.mirror.inc = add nuw i8 %idx.mirror, 1
>>> +  call void @use.i8(i8 %idx.mirror.inc)
>>> +
>>> +  %idx.inc = add i8 %idx, 1
>>> +; CHECK-NOT: %indvars.iv.next = add nuw nsw i32 %indvars.iv, 1
>>> +  %cmp = icmp ugt i8 %idx.inc, 0
>>> +  br i1 %cmp, label %loop, label %exit
>>> +
>>> + exit:
>>> +  ret void
>>> +}
>>>
>>> Modified: llvm/trunk/test/Analysis/ScalarEvolution/zext-signed-addrec.ll
>>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Analysis/ScalarEvolution/zext-signed-addrec.ll?rev=230280&r1=230279&r2=230280&view=diff
>>> ==============================================================================
>>> --- llvm/trunk/test/Analysis/ScalarEvolution/zext-signed-addrec.ll (original)
>>> +++ llvm/trunk/test/Analysis/ScalarEvolution/zext-signed-addrec.ll Mon Feb 23 17:22:58 2015
>>> @@ -43,7 +43,7 @@ if.end:
>>>    %shl = and i32 %conv7, 510
>>>    store i32 %shl, i32* @c, align 4
>>>
>>> -; CHECK: %lsr.iv.next = add i32 %lsr.iv, -258
>>> +; CHECK: %lsr.iv.next = add nsw i32 %lsr.iv, -258
>>>    %dec = add i8 %2, -1
>>>
>>>    %cmp2 = icmp sgt i8 %dec, -1
>>>
>>> Modified: llvm/trunk/test/CodeGen/AArch64/arm64-scaled_iv.ll
>>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/AArch64/arm64-scaled_iv.ll?rev=230280&r1=230279&r2=230280&view=diff
>>> ==============================================================================
>>> --- llvm/trunk/test/CodeGen/AArch64/arm64-scaled_iv.ll (original)
>>> +++ llvm/trunk/test/CodeGen/AArch64/arm64-scaled_iv.ll Mon Feb 23 17:22:58 2015
>>> @@ -20,7 +20,7 @@ for.body:
>>>    %arrayidx = getelementptr inbounds double* %b, i64 %tmp
>>>    %tmp1 = load double* %arrayidx, align 8
>>>  ; The induction variable should carry the scaling factor: 1 * 8 = 8.
>>> -; CHECK: [[IVNEXT]] = add nuw i64 [[IV]], 8
>>> +; CHECK: [[IVNEXT]] = add nuw nsw i64 [[IV]], 8
>>>    %indvars.iv.next = add i64 %indvars.iv, 1
>>>    %arrayidx2 = getelementptr inbounds double* %c, i64 %indvars.iv.next
>>>    %tmp2 = load double* %arrayidx2, align 8
>>>
>>> Modified: llvm/trunk/test/CodeGen/X86/avoid_complex_am.ll
>>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/avoid_complex_am.ll?rev=230280&r1=230279&r2=230280&view=diff
>>> ==============================================================================
>>> --- llvm/trunk/test/CodeGen/X86/avoid_complex_am.ll (original)
>>> +++ llvm/trunk/test/CodeGen/X86/avoid_complex_am.ll Mon Feb 23 17:22:58 2015
>>> @@ -22,7 +22,7 @@ for.body:
>>>    %arrayidx = getelementptr inbounds double* %b, i64 %tmp
>>>    %tmp1 = load double* %arrayidx, align 8
>>>  ; The induction variable should carry the scaling factor: 1.
>>> -; CHECK: [[IVNEXT]] = add nuw i64 [[IV]], 1
>>> +; CHECK: [[IVNEXT]] = add nuw nsw i64 [[IV]], 1
>>>    %indvars.iv.next = add i64 %indvars.iv, 1
>>>    %arrayidx2 = getelementptr inbounds double* %c, i64 %indvars.iv.next
>>>    %tmp2 = load double* %arrayidx2, align 8
>>>
>>> Modified: llvm/trunk/test/Transforms/IndVarSimplify/overflowcheck.ll
>>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/IndVarSimplify/overflowcheck.ll?rev=230280&r1=230279&r2=230280&view=diff
>>> ==============================================================================
>>> --- llvm/trunk/test/Transforms/IndVarSimplify/overflowcheck.ll (original)
>>> +++ llvm/trunk/test/Transforms/IndVarSimplify/overflowcheck.ll Mon Feb 23 17:22:58 2015
>>> @@ -9,7 +9,7 @@ target triple = "x86_64-apple-macosx"
>>>  ; CHECK: @llvm.sadd.with.overflow
>>>  ; CHECK-LABEL: loop2:
>>>  ; CHECK-NOT: extractvalue
>>> -; CHECK: add nuw nsw
>>> +; CHECK: add nuw
>>>  ; CHECK: @llvm.sadd.with.overflow
>>>  ; CHECK-LABEL: loop3:
>>>  ; CHECK-NOT: extractvalue
>>>
>>> Modified: llvm/trunk/test/Transforms/IndVarSimplify/pr20680.ll
>>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/IndVarSimplify/pr20680.ll?rev=230280&r1=230279&r2=230280&view=diff
>>> ==============================================================================
>>> --- llvm/trunk/test/Transforms/IndVarSimplify/pr20680.ll (original)
>>> +++ llvm/trunk/test/Transforms/IndVarSimplify/pr20680.ll Mon Feb 23 17:22:58 2015
>>> @@ -204,8 +204,8 @@ for.cond2.for.inc13_crit_edge:
>>>    br label %for.inc13
>>>
>>>  ; CHECK: [[for_inc13]]:
>>> -; CHECK-NEXT: %[[indvars_iv_next]] = add nuw nsw i32 %[[indvars_iv]], 1
>>> -; CHECK-NEXT: %[[exitcond4:.*]] = icmp ne i32 %[[indvars_iv]], -1
>>> +; CHECK-NEXT: %[[indvars_iv_next]] = add nsw i32 %[[indvars_iv]], 1
>>> +; CHECK-NEXT: %[[exitcond4:.*]] = icmp ne i32 %[[indvars_iv_next]], 0
>>>  ; CHECK-NEXT: br i1 %[[exitcond4]], label %[[for_cond2_preheader]], label %[[for_end15:.*]]
>>>  for.inc13:                                        ; preds = %for.cond2.for.inc13_crit_edge, %for.cond2.preheader
>>>    %inc14 = add i8 %storemerge15, 1
>>>
>>> Modified: llvm/trunk/test/Transforms/LoopStrengthReduce/count-to-zero.ll
>>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/LoopStrengthReduce/count-to-zero.ll?rev=230280&r1=230279&r2=230280&view=diff
>>> ==============================================================================
>>> --- llvm/trunk/test/Transforms/LoopStrengthReduce/count-to-zero.ll (original)
>>> +++ llvm/trunk/test/Transforms/LoopStrengthReduce/count-to-zero.ll Mon Feb 23 17:22:58 2015
>>> @@ -19,7 +19,7 @@ bb3:
>>>    %tmp4 = add i32 %c_addr.1, -1                   ; <i32> [#uses=1]
>>>    %c_addr.1.be = select i1 %tmp2, i32 %tmp3, i32 %tmp4 ; <i32> [#uses=1]
>>>    %indvar.next = add i32 %indvar, 1               ; <i32> [#uses=1]
>>> -; CHECK: add i32 %lsr.iv, -1
>>> +; CHECK: add nsw i32 %lsr.iv, -1
>>>    br label %bb6
>>>
>>>  bb6:                                              ; preds = %bb3, %entry
>>>
>>> Modified: llvm/trunk/test/Transforms/LoopStrengthReduce/uglygep.ll
>>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/LoopStrengthReduce/uglygep.ll?rev=230280&r1=230279&r2=230280&view=diff
>>> ==============================================================================
>>> --- llvm/trunk/test/Transforms/LoopStrengthReduce/uglygep.ll (original)
>>> +++ llvm/trunk/test/Transforms/LoopStrengthReduce/uglygep.ll Mon Feb 23 17:22:58 2015
>>> @@ -59,7 +59,7 @@ bb:
>>>  ; CHECK: loop0:
>>>  ; Induction variable is initialized to -2.
>>>  ; CHECK-NEXT: [[PHIIV:%[^ ]+]] = phi i32 [ [[IVNEXT:%[^ ]+]], %loop0 ], [ -2, %bb ]
>>> -; CHECK-NEXT: [[IVNEXT]] = add i32 [[PHIIV]], 1
>>> +; CHECK-NEXT: [[IVNEXT]] = add nuw nsw i32 [[PHIIV]], 1
>>>  ; CHECK-NEXT: br i1 false, label %loop0, label %bb0
>>>  loop0:                                            ; preds = %loop0, %bb
>>>    %i0 = phi i32 [ %i0.next, %loop0 ], [ 0, %bb ]  ; <i32> [#uses=2]
>>>
>>>
>>> _______________________________________________
>>> llvm-commits mailing list
>>> llvm-commits at cs.uiuc.edu
>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>
>>
>>
>> --
>> H.J.
>> _______________________________________________
>> 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