Skip to content

Commit

Permalink
[IndVarSimplify] Fixup nowrap flags during LFTR (PR31181)
Browse files Browse the repository at this point in the history
Fix for https://bugs.llvm.org/show_bug.cgi?id=31181 and partial fix
for LFTR poison handling issues in general.

When LFTR moves a condition from pre-inc to post-inc, it may now
depend on value that is poison due to nowrap flags. To avoid this,
we clear any nowrap flag that SCEV cannot prove for the post-inc
addrec.

Additionally, LFTR may switch to a different IV that is dynamically
dead and as such may be arbitrarily poison. This patch will correct
nowrap flags in some but not all cases where this happens. This is
related to the adoption of IR nowrap flags for the pre-inc addrec.
(See some of the switch_to_different_iv tests, where flags are not
dropped or insufficiently dropped.)

Finally, there are likely similar issues with the handling of GEP
inbounds, but we don't have a test case for this yet.

Differential Revision: https://reviews.llvm.org/D60935

llvm-svn: 362292
  • Loading branch information
nikic committed Jun 1, 2019
1 parent 2b1d799 commit 46d4dba
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 6 deletions.
21 changes: 21 additions & 0 deletions llvm/lib/Transforms/Scalar/IndVarSimplify.cpp
Expand Up @@ -2372,6 +2372,27 @@ linearFunctionTestReplace(Loop *L, const SCEV *BackedgeTakenCount,
CmpIndVar = IndVar->getIncomingValueForBlock(L->getExitingBlock());
}

// It may be necessary to drop nowrap flags on the incrementing instruction
// if either LFTR moves from a pre-inc check to a post-inc check (in which
// case the increment might have previously been poison on the last iteration
// only) or if LFTR switches to a different IV that was previously dynamically
// dead (and as such may be arbitrarily poison). We remove any nowrap flags
// that SCEV didn't infer for the post-inc addrec (even if we use a pre-inc
// check), because the pre-inc addrec flags may be adopted from the original
// instruction, while SCEV has to explicitly prove the post-inc nowrap flags.
// TODO: This handling is inaccurate for one case: If we switch to a
// dynamically dead IV that wraps on the first loop iteration only, which is
// not covered by the post-inc addrec. (If the new IV was not dynamically
// dead, it could not be poison on the first iteration in the first place.)
Value *IncVar = IndVar->getIncomingValueForBlock(L->getLoopLatch());
if (auto *BO = dyn_cast<BinaryOperator>(IncVar)) {
const SCEVAddRecExpr *AR = cast<SCEVAddRecExpr>(SE->getSCEV(IncVar));
if (BO->hasNoUnsignedWrap())
BO->setHasNoUnsignedWrap(AR->hasNoUnsignedWrap());
if (BO->hasNoSignedWrap())
BO->setHasNoSignedWrap(AR->hasNoSignedWrap());
}

Value *ExitCnt = genLoopLimit(IndVar, IVCount, L, Rewriter, SE);
assert(ExitCnt->getType()->isPointerTy() ==
IndVar->getType()->isPointerTy() &&
Expand Down
12 changes: 6 additions & 6 deletions llvm/test/Transforms/IndVarSimplify/lftr-pr31181.ll
Expand Up @@ -15,7 +15,7 @@ define i32 @test_drop_nuw() {
; CHECK: loop:
; CHECK-NEXT: [[STOREMERGE:%.*]] = phi i32 [ -2, [[ENTRY:%.*]] ], [ [[INC:%.*]], [[LOOP]] ]
; CHECK-NEXT: store i32 [[STOREMERGE]], i32* @a
; CHECK-NEXT: [[INC]] = add nuw nsw i32 [[STOREMERGE]], 1
; CHECK-NEXT: [[INC]] = add nsw i32 [[STOREMERGE]], 1
; CHECK-NEXT: [[EXITCOND:%.*]] = icmp ne i32 [[INC]], 0
; CHECK-NEXT: br i1 [[EXITCOND]], label [[LOOP]], label [[EXIT:%.*]]
; CHECK: exit:
Expand All @@ -42,7 +42,7 @@ define i32 @test_drop_nsw() {
; CHECK: loop:
; CHECK-NEXT: [[STOREMERGE:%.*]] = phi i32 [ 0, [[ENTRY:%.*]] ], [ [[INC:%.*]], [[LOOP]] ]
; CHECK-NEXT: store i32 [[STOREMERGE]], i32* @a
; CHECK-NEXT: [[INC]] = add nuw nsw i32 [[STOREMERGE]], 1
; CHECK-NEXT: [[INC]] = add nuw i32 [[STOREMERGE]], 1
; CHECK-NEXT: [[EXITCOND:%.*]] = icmp ne i32 [[INC]], -2147483648
; CHECK-NEXT: br i1 [[EXITCOND]], label [[LOOP]], label [[EXIT:%.*]]
; CHECK: exit:
Expand Down Expand Up @@ -155,7 +155,7 @@ define i32 @test_drop_nsw_var_lim(i32 %lim) {
; CHECK: loop:
; CHECK-NEXT: [[STOREMERGE:%.*]] = phi i32 [ [[INC:%.*]], [[LOOP]] ], [ 0, [[LOOP_PREHEADER]] ]
; CHECK-NEXT: store i32 [[STOREMERGE]], i32* @a
; CHECK-NEXT: [[INC]] = add nuw nsw i32 [[STOREMERGE]], 1
; CHECK-NEXT: [[INC]] = add nuw i32 [[STOREMERGE]], 1
; CHECK-NEXT: [[EXITCOND:%.*]] = icmp ne i32 [[INC]], [[TMP0]]
; CHECK-NEXT: br i1 [[EXITCOND]], label [[LOOP]], label [[EXIT_LOOPEXIT:%.*]]
; CHECK: exit.loopexit:
Expand Down Expand Up @@ -194,7 +194,7 @@ define i32 @switch_to_different_iv_post_inc(i32* %ptr, i1 %always_false) {
; CHECK-NEXT: br label [[ALWAYS_TAKEN]]
; CHECK: always_taken:
; CHECK-NEXT: [[IV_INC]] = add nsw i32 [[IV]], 1
; CHECK-NEXT: [[IV2_INC]] = add nuw nsw i32 [[IV2]], 1
; CHECK-NEXT: [[IV2_INC]] = add nuw i32 [[IV2]], 1
; CHECK-NEXT: [[EXITCOND:%.*]] = icmp ne i32 [[IV2_INC]], -2147483627
; CHECK-NEXT: br i1 [[EXITCOND]], label [[FOR_COND]], label [[FOR_END:%.*]]
; CHECK: for.end:
Expand Down Expand Up @@ -242,7 +242,7 @@ define i32 @switch_to_different_iv_pre_inc(i32* %ptr, i1 %always_false) {
; CHECK-NEXT: br label [[ALWAYS_TAKEN]]
; CHECK: always_taken:
; CHECK-NEXT: [[IV_INC]] = add nsw i32 [[IV]], 1
; CHECK-NEXT: [[IV2_INC]] = add nuw nsw i32 [[IV2]], 1
; CHECK-NEXT: [[IV2_INC]] = add nuw i32 [[IV2]], 1
; CHECK-NEXT: br label [[FOR_COND]]
; CHECK: for.end:
; CHECK-NEXT: ret i32 0
Expand Down Expand Up @@ -329,7 +329,7 @@ define i32 @switch_to_different_iv_second_poison(i32* %ptr, i1 %always_false) {
; CHECK-NEXT: store volatile i32 [[IV2]], i32* [[PTR]]
; CHECK-NEXT: br label [[ALWAYS_TAKEN]]
; CHECK: always_taken:
; CHECK-NEXT: [[IV2_INC]] = add nuw nsw i32 [[IV2]], 1
; CHECK-NEXT: [[IV2_INC]] = add nsw i32 [[IV2]], 1
; CHECK-NEXT: [[IV_INC]] = add nsw i32 [[IV]], 1
; CHECK-NEXT: [[EXITCOND:%.*]] = icmp ne i32 [[IV2_INC]], -2147483629
; CHECK-NEXT: br i1 [[EXITCOND]], label [[FOR_COND]], label [[FOR_END:%.*]]
Expand Down

0 comments on commit 46d4dba

Please sign in to comment.