Skip to content

Commit e46d74b

Browse files
committed
[CVP] Allow two transforms in one invocation
For a call site which had both constant deopt operands and nonnull arguments, we were missing the opportunity to recognize the later by bailing early. This is somewhat of a speculative fix. Months ago, I'd had a private report of performance and compile time regressions from the deopt operand folding. I never received a test case. However, the only possibility I see was that after that change CVP missed the nonnull fold, and we end up with a pass ordering/missed simplification issue. So, since it's a real issue, fix it and hope.
1 parent bd08a87 commit e46d74b

File tree

2 files changed

+29
-7
lines changed

2 files changed

+29
-7
lines changed

llvm/lib/Transforms/Scalar/CorrelatedValuePropagation.cpp

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -524,8 +524,6 @@ static void processSaturatingInst(SaturatingInst *SI, LazyValueInfo *LVI) {
524524

525525
/// Infer nonnull attributes for the arguments at the specified callsite.
526526
static bool processCallSite(CallBase &CB, LazyValueInfo *LVI) {
527-
SmallVector<unsigned, 4> ArgNos;
528-
unsigned ArgNo = 0;
529527

530528
if (auto *WO = dyn_cast<WithOverflowInst>(&CB)) {
531529
if (WO->getLHS()->getType()->isIntegerTy() && willNotOverflow(WO, LVI)) {
@@ -541,6 +539,8 @@ static bool processCallSite(CallBase &CB, LazyValueInfo *LVI) {
541539
}
542540
}
543541

542+
bool Changed = false;
543+
544544
// Deopt bundle operands are intended to capture state with minimal
545545
// perturbance of the code otherwise. If we can find a constant value for
546546
// any such operand and remove a use of the original value, that's
@@ -549,7 +549,6 @@ static bool processCallSite(CallBase &CB, LazyValueInfo *LVI) {
549549
// idiomatically, appear along rare conditional paths, it's reasonable likely
550550
// we may have a conditional fact with which LVI can fold.
551551
if (auto DeoptBundle = CB.getOperandBundle(LLVMContext::OB_deopt)) {
552-
bool Progress = false;
553552
for (const Use &ConstU : DeoptBundle->Inputs) {
554553
Use &U = const_cast<Use&>(ConstU);
555554
Value *V = U.get();
@@ -559,12 +558,13 @@ static bool processCallSite(CallBase &CB, LazyValueInfo *LVI) {
559558
Constant *C = LVI->getConstant(V, &CB);
560559
if (!C) continue;
561560
U.set(C);
562-
Progress = true;
561+
Changed = true;
563562
}
564-
if (Progress)
565-
return true;
566563
}
567564

565+
SmallVector<unsigned, 4> ArgNos;
566+
unsigned ArgNo = 0;
567+
568568
for (Value *V : CB.args()) {
569569
PointerType *Type = dyn_cast<PointerType>(V->getType());
570570
// Try to mark pointer typed parameters as non-null. We skip the
@@ -582,7 +582,7 @@ static bool processCallSite(CallBase &CB, LazyValueInfo *LVI) {
582582
assert(ArgNo == CB.arg_size() && "sanity check");
583583

584584
if (ArgNos.empty())
585-
return false;
585+
return Changed;
586586

587587
AttributeList AS = CB.getAttributes();
588588
LLVMContext &Ctx = CB.getContext();

llvm/test/Transforms/CorrelatedValuePropagation/deopt.ll

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@
22
; RUN: opt -correlated-propagation -S < %s | FileCheck %s
33

44
declare void @use()
5+
declare void @use_ptr(i8*)
6+
57
; test requires a mix of context sensative refinement, and analysis
68
; of the originating IR pattern. Neither part is enough in isolation.
79
define void @test1(i1 %c, i1 %c2) {
@@ -140,3 +142,23 @@ taken:
140142
untaken:
141143
ret void
142144
}
145+
146+
147+
define void @test5(i64 %a, i8* nonnull %p) {
148+
; CHECK-LABEL: @test5(
149+
; CHECK-NEXT: [[CMP:%.*]] = icmp eq i64 [[A:%.*]], 0
150+
; CHECK-NEXT: br i1 [[CMP]], label [[TAKEN:%.*]], label [[UNTAKEN:%.*]]
151+
; CHECK: taken:
152+
; CHECK-NEXT: call void @use_ptr(i8* nonnull [[P:%.*]]) [ "deopt"(i64 0) ]
153+
; CHECK-NEXT: ret void
154+
; CHECK: untaken:
155+
; CHECK-NEXT: ret void
156+
;
157+
%cmp = icmp eq i64 %a, 0
158+
br i1 %cmp, label %taken, label %untaken
159+
taken:
160+
call void @use_ptr(i8* %p) ["deopt" (i64 %a)]
161+
ret void
162+
untaken:
163+
ret void
164+
}

0 commit comments

Comments
 (0)