Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions llvm/include/llvm/Analysis/ScalarEvolution.h
Original file line number Diff line number Diff line change
Expand Up @@ -1761,6 +1761,10 @@ class ScalarEvolution {
/// V.
const SCEV *getOperandsToCreate(Value *V, SmallVectorImpl<Value *> &Ops);

/// Returns SCEV for the first operand of a phi if all phi operands have
/// identical opcodes and operands
const SCEV *createNodeForPHIWithIdenticalOperands(PHINode *PN);

/// Provide the special handling we need to analyze PHI SCEVs.
const SCEV *createNodeForPHI(PHINode *PN);

Expand Down
31 changes: 31 additions & 0 deletions llvm/lib/Analysis/ScalarEvolution.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6019,6 +6019,34 @@ const SCEV *ScalarEvolution::createNodeFromSelectLikePHI(PHINode *PN) {
return nullptr;
}

/// Returns SCEV for the first operand of a phi if all phi operands have
/// identical opcodes and operands
/// eg.
/// a: %add = %a + %b
/// br %c
/// b: %add1 = %a + %b
/// br %c
/// c: %phi = phi [%add, a], [%add1, b]
/// scev(%phi) => scev(%add)
const SCEV *
ScalarEvolution::createNodeForPHIWithIdenticalOperands(PHINode *PN) {
BinaryOperator *CommonInst = nullptr;
for (Value *Incoming : PN->incoming_values()) {
BinaryOperator *IncomingInst = dyn_cast<BinaryOperator>(Incoming);
if (CommonInst) {
if (!(IncomingInst && CommonInst->isIdenticalTo(IncomingInst)))
return nullptr; // Not identical, give up
} else if (IncomingInst) {
// Remember binary operator
CommonInst = IncomingInst;
} else {
// Not a binary operator, give up
return nullptr;
}
}
return CommonInst ? getSCEV(CommonInst) : nullptr;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks dangerous to me. Even if the instructions are the same, I think that SCEV would be allowed to use context-sensitive reasoning when constructing the SCEV node. It would be safer to call getSCEV for each incoming value in a second loop and make sure they're all the same.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using a single loop with calls to getSCEV.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I expect that you do need both loops -- not for correctness, but as a profitability heuristic. Computing SCEVs is expensive, and always recursing through phis would likely add significant cost.

I tried to confirm this but the stage2 build crashes (https://llvm-compile-time-tracker.com/show_error.php?commit=a4e3a0e648d7a3664ca6269e846abf2e6ddcdb08). I didn't investigate, but it might be that the recursion causes a stack overflow.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Restored the original loop, and added an all_of check for SCEV exprs of the incoming values being identical.

}

const SCEV *ScalarEvolution::createNodeForPHI(PHINode *PN) {
if (const SCEV *S = createAddRecFromPHI(PN))
return S;
Expand All @@ -6030,6 +6058,9 @@ const SCEV *ScalarEvolution::createNodeForPHI(PHINode *PN) {
/*UseInstrInfo=*/true, /*CanUseUndef=*/false}))
return getSCEV(V);

if (const SCEV *S = createNodeForPHIWithIdenticalOperands(PN))
return S;

if (const SCEV *S = createNodeFromSelectLikePHI(PN))
return S;

Expand Down
46 changes: 46 additions & 0 deletions llvm/test/Analysis/ScalarEvolution/trip-count16.ll
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
; NOTE: Assertions have been autogenerated by utils/update_analyze_test_checks.py UTC_ARGS: --version 5
; RUN: opt < %s -disable-output "-passes=print<scalar-evolution>" 2>&1 | FileCheck %s
define void @test(ptr %x, ptr %y) {
; CHECK-LABEL: 'test'
; CHECK-NEXT: Classifying expressions for: @test
; CHECK-NEXT: %v1.0 = phi i32 [ 0, %entry ], [ %k.0, %if.end ]
; CHECK-NEXT: --> {0,+,1}<nuw><nsw><%for.cond> U: [0,7) S: [0,7) Exits: 6 LoopDispositions: { %for.cond: Computable }
; CHECK-NEXT: %add = add nsw i32 %v1.0, 1
; CHECK-NEXT: --> {1,+,1}<nuw><nsw><%for.cond> U: [1,8) S: [1,8) Exits: 7 LoopDispositions: { %for.cond: Computable }
; CHECK-NEXT: %add6 = add nsw i32 %v1.0, 1
; CHECK-NEXT: --> {1,+,1}<nuw><nsw><%for.cond> U: [1,8) S: [1,8) Exits: 7 LoopDispositions: { %for.cond: Computable }
; CHECK-NEXT: %k.0 = phi i32 [ %add, %if.then ], [ %add6, %if.else ]
; CHECK-NEXT: --> {1,+,1}<nuw><nsw><%for.cond> U: [1,8) S: [1,8) Exits: 7 LoopDispositions: { %for.cond: Computable }
; CHECK-NEXT: Determining loop execution counts for: @test
; CHECK-NEXT: Loop %for.cond: backedge-taken count is i32 6
; CHECK-NEXT: Loop %for.cond: constant max backedge-taken count is i32 6
; CHECK-NEXT: Loop %for.cond: symbolic max backedge-taken count is i32 6
; CHECK-NEXT: Loop %for.cond: Trip multiple is 7
;
entry:
br label %for.cond

for.cond: ; preds = %6, %0
%v1.0 = phi i32 [ 0, %entry ], [ %k.0, %if.end ]
%cmp = icmp slt i32 %v1.0, 6
br i1 %cmp, label %for.body, label %exit

for.body: ; preds = %1
%cmp3 = icmp slt i32 %v1.0, 2
br i1 %cmp3, label %if.then, label %if.else

if.then: ; preds = %2
%add = add nsw i32 %v1.0, 1
br label %if.end

if.else: ; preds = %2
%add6 = add nsw i32 %v1.0, 1
br label %if.end

if.end: ; preds = %4, %3
%k.0 = phi i32 [ %add, %if.then ], [ %add6, %if.else ]
br label %for.cond

exit: ; preds = %5
ret void
}
Loading