[llvm] 1a5f4cb - [InstCombine] Add optimization to prevent poison from being propagated.
Juneyoung Lee via llvm-commits
llvm-commits at lists.llvm.org
Sat Jul 10 20:41:20 PDT 2021
Author: hyeongyu kim
Date: 2021-07-11T12:40:43+09:00
New Revision: 1a5f4cbe1bd62e6624cbb77dad0d363addd1b324
URL: https://github.com/llvm/llvm-project/commit/1a5f4cbe1bd62e6624cbb77dad0d363addd1b324
DIFF: https://github.com/llvm/llvm-project/commit/1a5f4cbe1bd62e6624cbb77dad0d363addd1b324.diff
LOG: [InstCombine] Add optimization to prevent poison from being propagated.
In D104569, Freeze was inserted just before br to solve the `branching on undef` miscompilation problem.
But value analysis was being disturbed by added freeze.
```
v = load ptr
cond = freeze(icmp (and v, const), const')
br cond, ...
```
The case in which value analysis disturbed is as above.
By changing freeze to add immediately after load, value analysis will be successful again.
```
v = load ptr
freeze(icmp (and v, const), const')
=>
v = load ptr
v' = freeze v
icmp (and v', const), const'
```
In this patch, I propose the above optimization.
With this patch, the poison will not spread as the freeze is performed early.
Reviewed By: nikic, lebedev.ri
Differential Revision: https://reviews.llvm.org/D105392
Added:
Modified:
llvm/lib/Transforms/InstCombine/InstCombineInternal.h
llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
llvm/test/Transforms/InstCombine/freeze.ll
Removed:
################################################################################
diff --git a/llvm/lib/Transforms/InstCombine/InstCombineInternal.h b/llvm/lib/Transforms/InstCombine/InstCombineInternal.h
index 896fa9fd290c2..76552801a7f95 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineInternal.h
+++ b/llvm/lib/Transforms/InstCombine/InstCombineInternal.h
@@ -168,6 +168,7 @@ class LLVM_LIBRARY_VISIBILITY InstCombinerImpl final
Instruction *visitExtractValueInst(ExtractValueInst &EV);
Instruction *visitLandingPadInst(LandingPadInst &LI);
Instruction *visitVAEndInst(VAEndInst &I);
+ Value *pushFreezeToPreventPoisonFromPropagating(FreezeInst &FI);
Instruction *visitFreeze(FreezeInst &I);
/// Specify what to return for unhandled instructions.
diff --git a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
index e00bcf8826d0d..7bbb4d512347b 100644
--- a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
@@ -3514,6 +3514,57 @@ Instruction *InstCombinerImpl::visitLandingPadInst(LandingPadInst &LI) {
return nullptr;
}
+Value *
+InstCombinerImpl::pushFreezeToPreventPoisonFromPropagating(FreezeInst &OrigFI) {
+ // Try to push freeze through instructions that propagate but don't produce
+ // poison as far as possible. If an operand of freeze follows three
+ // conditions 1) one-use, 2) does not produce poison, and 3) has all but one
+ // guaranteed-non-poison operands then push the freeze through to the one
+ // operand that is not guaranteed non-poison. The actual transform is as
+ // follows.
+ // Op1 = ... ; Op1 can be posion
+ // Op0 = Inst(Op1, NonPoisonOps...) ; Op0 has only one use and only have
+ // ; single guaranteed-non-poison operands
+ // ... = Freeze(Op0)
+ // =>
+ // Op1 = ...
+ // Op1.fr = Freeze(Op1)
+ // ... = Inst(Op1.fr, NonPoisonOps...)
+ auto *OrigOp = OrigFI.getOperand(0);
+ auto *OrigOpInst = dyn_cast<Instruction>(OrigOp);
+
+ // While we could change the other users of OrigOp to use freeze(OrigOp), that
+ // potentially reduces their optimization potential, so let's only do this iff
+ // the OrigOp is only used by the freeze.
+ if (!OrigOpInst || !OrigOpInst->hasOneUse() || isa<PHINode>(OrigOp) ||
+ canCreateUndefOrPoison(dyn_cast<Operator>(OrigOp)))
+ return nullptr;
+
+ // If operand is guaranteed not to be poison, there is no need to add freeze
+ // to the operand. So we first find the operand that is not guaranteed to be
+ // poison.
+ Use *MaybePoisonOperand = nullptr;
+ for (Use &U : OrigOpInst->operands()) {
+ if (isGuaranteedNotToBeUndefOrPoison(U.get()))
+ continue;
+ if (!MaybePoisonOperand)
+ MaybePoisonOperand = &U;
+ else
+ return nullptr;
+ }
+
+ // If all operands are guaranteed to be non-poison, we can drop freeze.
+ if (!MaybePoisonOperand)
+ return OrigOp;
+
+ auto *FrozenMaybePoisonOperand = new FreezeInst(
+ MaybePoisonOperand->get(), MaybePoisonOperand->get()->getName() + ".fr");
+
+ replaceUse(*MaybePoisonOperand, FrozenMaybePoisonOperand);
+ FrozenMaybePoisonOperand->insertBefore(OrigOpInst);
+ return OrigOp;
+}
+
Instruction *InstCombinerImpl::visitFreeze(FreezeInst &I) {
Value *Op0 = I.getOperand(0);
@@ -3526,6 +3577,9 @@ Instruction *InstCombinerImpl::visitFreeze(FreezeInst &I) {
return NV;
}
+ if (Value *NI = pushFreezeToPreventPoisonFromPropagating(I))
+ return replaceInstUsesWith(I, NI);
+
if (match(Op0, m_Undef())) {
// If I is freeze(undef), see its uses and fold it to the best constant.
// - or: pick -1
diff --git a/llvm/test/Transforms/InstCombine/freeze.ll b/llvm/test/Transforms/InstCombine/freeze.ll
index 2546ec387003f..be2d6146c054c 100644
--- a/llvm/test/Transforms/InstCombine/freeze.ll
+++ b/llvm/test/Transforms/InstCombine/freeze.ll
@@ -86,3 +86,52 @@ define void @or_select_multipleuses_logical(i32 %x, i1 %y) {
call void @use_i32_i1(i32 %a, i1 %b)
ret void
}
+
+; Move the freeze forward to prevent poison from spreading.
+
+define i32 @early_freeze_test1(i32 %x, i32 %y) {
+; CHECK-LABEL: @early_freeze_test1(
+; CHECK-NEXT: [[V1:%.*]] = add i32 [[X:%.*]], [[Y:%.*]]
+; CHECK-NEXT: [[V1_FR:%.*]] = freeze i32 [[V1]]
+; CHECK-NEXT: [[V2:%.*]] = shl i32 [[V1_FR]], 1
+; CHECK-NEXT: [[V3:%.*]] = and i32 [[V2]], 2
+; CHECK-NEXT: ret i32 [[V3]]
+;
+ %v1 = add i32 %x, %y
+ %v2 = shl i32 %v1, 1
+ %v3 = and i32 %v2, 2
+ %v3.fr = freeze i32 %v3
+ ret i32 %v3.fr
+}
+
+define i1 @early_freeze_test2(i32* %ptr) {
+; CHECK-LABEL: @early_freeze_test2(
+; CHECK-NEXT: [[V1:%.*]] = load i32, i32* [[PTR:%.*]], align 4
+; CHECK-NEXT: [[V1_FR:%.*]] = freeze i32 [[V1]]
+; CHECK-NEXT: [[V2:%.*]] = and i32 [[V1_FR]], 1
+; CHECK-NEXT: [[COND:%.*]] = icmp eq i32 [[V2]], 0
+; CHECK-NEXT: ret i1 [[COND]]
+;
+ %v1 = load i32, i32* %ptr
+ %v2 = and i32 %v1, 1
+ %cond = icmp eq i32 %v2, 0
+ %cond.fr = freeze i1 %cond
+ ret i1 %cond.fr
+}
+
+; add can overflows, so we cannot move freeze beyond add
+
+define i32 @early_freeze_test3(i32 %v1) {
+; CHECK-LABEL: @early_freeze_test3(
+; CHECK-NEXT: [[V2:%.*]] = shl i32 [[V1:%.*]], 1
+; CHECK-NEXT: [[V3:%.*]] = add nuw i32 [[V2]], 2
+; CHECK-NEXT: [[V3_FR:%.*]] = freeze i32 [[V3]]
+; CHECK-NEXT: [[V4:%.*]] = or i32 [[V3_FR]], 1
+; CHECK-NEXT: ret i32 [[V4]]
+;
+ %v2 = shl i32 %v1, 1
+ %v3 = add nuw i32 %v2, 2
+ %v4 = or i32 %v3, 1
+ %v4.fr = freeze i32 %v4
+ ret i32 %v4.fr
+}
More information about the llvm-commits
mailing list