Skip to content

Commit

Permalink
[IndVars] Set Changed if sinkUnusedInvariants changes IR. PR38863
Browse files Browse the repository at this point in the history
Currently, `sinkUnusedInvariants` does not set Changed flag even if it makes
changes in the IR. There is no clear evidence that it can cause a crash, but it
looks highly suspicious and likely invalid.

Differential Revision: https://reviews.llvm.org/D51777
Reviewed By: skatkov

llvm-svn: 341777
  • Loading branch information
Max Kazantsev committed Sep 10, 2018
1 parent 09be521 commit 4d10ba3
Show file tree
Hide file tree
Showing 2 changed files with 41 additions and 5 deletions.
14 changes: 9 additions & 5 deletions llvm/lib/Transforms/Scalar/IndVarSimplify.cpp
Expand Up @@ -150,7 +150,7 @@ class IndVarSimplify {
Value *linearFunctionTestReplace(Loop *L, const SCEV *BackedgeTakenCount,
PHINode *IndVar, SCEVExpander &Rewriter);

void sinkUnusedInvariants(Loop *L);
bool sinkUnusedInvariants(Loop *L);

public:
IndVarSimplify(LoopInfo *LI, ScalarEvolution *SE, DominatorTree *DT,
Expand Down Expand Up @@ -2476,13 +2476,14 @@ linearFunctionTestReplace(Loop *L,
/// If there's a single exit block, sink any loop-invariant values that
/// were defined in the preheader but not used inside the loop into the
/// exit block to reduce register pressure in the loop.
void IndVarSimplify::sinkUnusedInvariants(Loop *L) {
bool IndVarSimplify::sinkUnusedInvariants(Loop *L) {
BasicBlock *ExitBlock = L->getExitBlock();
if (!ExitBlock) return;
if (!ExitBlock) return false;

BasicBlock *Preheader = L->getLoopPreheader();
if (!Preheader) return;
if (!Preheader) return false;

bool MadeAnyChanges = false;
BasicBlock::iterator InsertPt = ExitBlock->getFirstInsertionPt();
BasicBlock::iterator I(Preheader->getTerminator());
while (I != Preheader->begin()) {
Expand Down Expand Up @@ -2552,10 +2553,13 @@ void IndVarSimplify::sinkUnusedInvariants(Loop *L) {
Done = true;
}

MadeAnyChanges = true;
ToMove->moveBefore(*ExitBlock, InsertPt);
if (Done) break;
InsertPt = ToMove->getIterator();
}

return MadeAnyChanges;
}

//===----------------------------------------------------------------------===//
Expand Down Expand Up @@ -2649,7 +2653,7 @@ bool IndVarSimplify::run(Loop *L) {

// Loop-invariant instructions in the preheader that aren't used in the
// loop may be sunk below the loop to reduce register pressure.
sinkUnusedInvariants(L);
Changed |= sinkUnusedInvariants(L);

// rewriteFirstIterationLoopExitValues does not rely on the computation of
// trip count and therefore can further simplify exit values in addition to
Expand Down
32 changes: 32 additions & 0 deletions llvm/test/Transforms/IndVarSimplify/sink-from-preheader.ll
@@ -0,0 +1,32 @@
; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
; RUN: opt < %s -indvars -S | FileCheck %s
target datalayout = "e-p:32:32:32-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:32:64-f32:32:32-f64:32:64-v64:64:64-v128:128:128-a0:0:64-f80:128:128"
target triple = "i386-apple-darwin10.0"

; We make sinking here, Changed flag should be set properly.
define i32 @test(i32 %a, i32 %b, i32 %N) {
; CHECK-LABEL: @test(
; CHECK-NEXT: entry:
; CHECK-NEXT: br label [[LOOP:%.*]]
; CHECK: loop:
; CHECK-NEXT: [[IV:%.*]] = phi i32 [ 0, [[ENTRY:%.*]] ], [ [[IV_NEXT:%.*]], [[LOOP]] ]
; CHECK-NEXT: [[IV_NEXT]] = add nuw nsw i32 [[IV]], 1
; CHECK-NEXT: [[CMP:%.*]] = icmp slt i32 [[IV_NEXT]], [[N:%.*]]
; CHECK-NEXT: br i1 [[CMP]], label [[LOOP]], label [[EXIT:%.*]]
; CHECK: exit:
; CHECK-NEXT: [[ADD:%.*]] = add i32 [[A:%.*]], [[B:%.*]]
; CHECK-NEXT: ret i32 [[ADD]]
;
entry:
%add = add i32 %a, %b
br label %loop

loop:
%iv = phi i32 [ 0, %entry ], [ %iv.next, %loop ]
%iv.next = add i32 %iv, 1
%cmp = icmp slt i32 %iv.next, %N
br i1 %cmp, label %loop, label %exit

exit:
ret i32 %add
}

0 comments on commit 4d10ba3

Please sign in to comment.