[PATCH] D144106: [InstSimplify] Fix poison safety in insertvalue fold

[Nikita Popov via Phabricator via llvm-commits]

nikic created this revision.
nikic added reviewers: spatel, nlopes.
Herald added subscribers: StephenFan, hiraditya.
Herald added a project: All.
nikic requested review of this revision.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

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.


https://reviews.llvm.org/D144106

Files:
  llvm/lib/Analysis/InstructionSimplify.cpp
  llvm/test/Transforms/Inline/pr50270.ll
  llvm/test/Transforms/InstSimplify/2011-09-05-InsertExtractValue.ll
  llvm/test/Transforms/InstSimplify/insertvalue.ll


Index: llvm/test/Transforms/InstSimplify/insertvalue.ll
===================================================================
--- llvm/test/Transforms/InstSimplify/insertvalue.ll
+++ llvm/test/Transforms/InstSimplify/insertvalue.ll
@@ -37,7 +37,9 @@
 
 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
Index: llvm/test/Transforms/InstSimplify/2011-09-05-InsertExtractValue.ll
===================================================================
--- llvm/test/Transforms/InstSimplify/2011-09-05-InsertExtractValue.ll
+++ llvm/test/Transforms/InstSimplify/2011-09-05-InsertExtractValue.ll
@@ -23,7 +23,7 @@
   %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
 }
Index: llvm/test/Transforms/Inline/pr50270.ll
===================================================================
--- llvm/test/Transforms/Inline/pr50270.ll
+++ llvm/test/Transforms/Inline/pr50270.ll
@@ -10,17 +10,17 @@
 
 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
@@ -36,16 +36,16 @@
 ; 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]]
 ;
@@ -59,7 +59,7 @@
   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
 }
Index: llvm/lib/Analysis/InstructionSimplify.cpp
===================================================================
--- llvm/lib/Analysis/InstructionSimplify.cpp
+++ llvm/lib/Analysis/InstructionSimplify.cpp
@@ -4892,8 +4892,11 @@
   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


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D144106.497675.patch
Type: text/x-patch
Size: 4563 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20230215/ff286ccf/attachment.bin>