[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
This commit is contained in:
hyeongyu kim 2021-07-11 11:45:32 +09:00 committed by Juneyoung Lee
parent 09cdcf09b5
commit 1a5f4cbe1b
3 changed files with 104 additions and 0 deletions

View File

@ -168,6 +168,7 @@ public:
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.

View File

@ -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

View File

@ -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
}