Skip to content

Commit

Permalink
[InstSimplify] Fix poison safety in insertvalue fold
Browse files Browse the repository at this point in the history
We can only fold insertvalue undef, (extractvalue x, n) to x
if x is not poison, otherwise we might be replacing undef with
poison (https://alive2.llvm.org/ce/z/fnw3c8). The insertvalue
poison case is always fine.

I didn't go to particularly large effort to preserve cases where
folding with undef is still legal (mainly when there is a chain of
multiple inserts that end up covering the whole aggregate),
because this shouldn't really occur in practice: We should always
be generating the insertvalue poison form when constructing
aggregates nowadays.

Differential Revision: https://reviews.llvm.org/D144106
  • Loading branch information
nikic committed Feb 16, 2023
1 parent b0de872 commit 9ca2c30
Show file tree
Hide file tree
Showing 4 changed files with 16 additions and 11 deletions.
7 changes: 5 additions & 2 deletions llvm/lib/Analysis/InstructionSimplify.cpp
Expand Up @@ -4892,8 +4892,11 @@ static Value *simplifyInsertValueInst(Value *Agg, Value *Val,
if (ExtractValueInst *EV = dyn_cast<ExtractValueInst>(Val))
if (EV->getAggregateOperand()->getType() == Agg->getType() &&
EV->getIndices() == Idxs) {
// insertvalue undef, (extractvalue y, n), n -> y
if (Q.isUndefValue(Agg))
// insertvalue poison, (extractvalue y, n), n -> y
// insertvalue undef, (extractvalue y, n), n -> y if y cannot be poison
if (isa<PoisonValue>(Agg) ||
(Q.isUndefValue(Agg) &&
isGuaranteedNotToBePoison(EV->getAggregateOperand())))
return EV->getAggregateOperand();

// insertvalue y, (extractvalue y, n), n -> y
Expand Down
14 changes: 7 additions & 7 deletions llvm/test/Transforms/Inline/pr50270.ll
Expand Up @@ -10,17 +10,17 @@ declare { ptr } @opaque_callee()

define { ptr } @callee(ptr %x) {
; CHECK-LABEL: @callee(
; CHECK-NEXT: [[RES:%.*]] = insertvalue { ptr } undef, ptr [[X:%.*]], 0
; CHECK-NEXT: [[RES:%.*]] = insertvalue { ptr } poison, ptr [[X:%.*]], 0
; CHECK-NEXT: ret { ptr } [[RES]]
;
%res = insertvalue { ptr } undef, ptr %x, 0
%res = insertvalue { ptr } poison, ptr %x, 0
ret { ptr } %res
}

; @opaque_callee() should not receive noalias metadata here.
define void @caller() {
; CHECK-LABEL: @caller(
; CHECK-NEXT: call void @llvm.experimental.noalias.scope.decl(metadata !0)
; CHECK-NEXT: call void @llvm.experimental.noalias.scope.decl(metadata [[META0:![0-9]+]])
; CHECK-NEXT: [[S:%.*]] = call { ptr } @opaque_callee()
; CHECK-NEXT: [[X:%.*]] = extractvalue { ptr } [[S]], 0
; CHECK-NEXT: ret void
Expand All @@ -36,16 +36,16 @@ define void @caller() {
; else branch, not as the load in the if branch.
define { ptr } @self_caller(i1 %c, ptr %a) {
; CHECK-LABEL: @self_caller(
; CHECK-NEXT: call void @llvm.experimental.noalias.scope.decl(metadata !0)
; CHECK-NEXT: call void @llvm.experimental.noalias.scope.decl(metadata [[META0]])
; CHECK-NEXT: br i1 [[C:%.*]], label [[IF:%.*]], label [[ELSE:%.*]]
; CHECK: if:
; CHECK-NEXT: [[S:%.*]] = call { ptr } @opaque_callee(), !noalias !0
; CHECK-NEXT: [[X:%.*]] = extractvalue { ptr } [[S]], 0
; CHECK-NEXT: call void @llvm.experimental.noalias.scope.decl(metadata !3)
; CHECK-NEXT: call void @llvm.experimental.noalias.scope.decl(metadata [[META3:![0-9]+]])
; CHECK-NEXT: [[TMP1:%.*]] = load volatile i64, ptr [[X]], align 4, !alias.scope !3
; CHECK-NEXT: ret { ptr } [[S]]
; CHECK: else:
; CHECK-NEXT: [[R2:%.*]] = insertvalue { ptr } undef, ptr [[A:%.*]], 0
; CHECK-NEXT: [[R2:%.*]] = insertvalue { ptr } poison, ptr [[A:%.*]], 0
; CHECK-NEXT: [[TMP2:%.*]] = load volatile i64, ptr [[A]], align 4, !alias.scope !0
; CHECK-NEXT: ret { ptr } [[R2]]
;
Expand All @@ -59,7 +59,7 @@ if:
ret { ptr } %r

else:
%r2 = insertvalue { ptr } undef, ptr %a, 0
%r2 = insertvalue { ptr } poison, ptr %a, 0
load volatile i64, ptr %a, !alias.scope !0
ret { ptr } %r2
}
Expand Down
Expand Up @@ -23,7 +23,7 @@ lpad:
%ex = landingpad { ptr, i32 } cleanup
%exc_ptr = extractvalue { ptr, i32 } %ex, 0
%filter = extractvalue { ptr, i32 } %ex, 1
%exc_ptr2 = insertvalue { ptr, i32 } undef, ptr %exc_ptr, 0
%exc_ptr2 = insertvalue { ptr, i32 } poison, ptr %exc_ptr, 0
%filter2 = insertvalue { ptr, i32 } %exc_ptr2, i32 %filter, 1
resume { ptr, i32 } %filter2
}
Expand Down
4 changes: 3 additions & 1 deletion llvm/test/Transforms/InstSimplify/insertvalue.ll
Expand Up @@ -37,7 +37,9 @@ define {i32, i32} @insert_into_poison({i32, i32} %x) {

define {i32, i32} @insert_into_undef({i32, i32} %x) {
; CHECK-LABEL: @insert_into_undef(
; CHECK-NEXT: ret { i32, i32 } [[X:%.*]]
; CHECK-NEXT: [[ELEM:%.*]] = extractvalue { i32, i32 } [[X:%.*]], 0
; CHECK-NEXT: [[V:%.*]] = insertvalue { i32, i32 } undef, i32 [[ELEM]], 0
; CHECK-NEXT: ret { i32, i32 } [[V]]
;
%elem = extractvalue {i32, i32} %x, 0
%v = insertvalue {i32, i32} undef, i32 %elem, 0
Expand Down

0 comments on commit 9ca2c30

Please sign in to comment.