[InstSimplify] Fix poison safety in insertvalue fold

[nikic]

Summary

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.

[nlopes]

This is great, thank you!

I think this is safe by now; we've removed most insertvalues with undef by now.

[spatel]

LGTM

llvm/lib/Analysis/InstructionSimplify.cpp

static Value *simplifyInsertValueInst(Value *Agg, Value *Val,
   if (isa<PoisonValue>(Val) ||
       (Q.isUndefValue(Val) && isGuaranteedNotToBePoison(Agg)))
     return Agg;
 
   // insertvalue x, (extractvalue y, n), n
   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
       if (Agg == EV->getAggregateOperand())
         return Agg;
     }
 
   return nullptr;

llvm/test/Transforms/Inline/pr50270.ll

 ; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
 ; RUN: opt -S -passes=inline < %s | FileCheck %s
 
 ; This tests cases where instructions in the callee are simplified to
 ; instructions in the caller, thus making VMap contain instructions from
 ; the caller. We should not be assigning incorrect noalias metadata in
 ; that case.
 
 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
 ;
   call void @llvm.experimental.noalias.scope.decl(metadata !0)
   %s = call { ptr } @opaque_callee()
   %x = extractvalue { ptr } %s, 0
   call { ptr } @callee(ptr %x), !noalias !0
   ret void
 }
 
 ; @opaque_callee() should no the same noalias metadata as the load from the
 ; 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]]
 ;
   call void @llvm.experimental.noalias.scope.decl(metadata !0)
   br i1 %c, label %if, label %else
 
 if:
   %s = call { ptr } @opaque_callee(), !noalias !0
   %x = extractvalue { ptr } %s, 0
   %r = call { ptr } @self_caller(i1 false, ptr %x)
   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
 }
 
 declare void @llvm.experimental.noalias.scope.decl(metadata)
 
 !0 = !{!1}
 !1 = !{!1, !2, !"scope"}
 !2 = !{!2, !"domain"}

llvm/test/Transforms/InstSimplify/2011-09-05-InsertExtractValue.ll

 entry:
   invoke void @bar() to label %cont unwind label %lpad
 cont:
   ret void
 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
 }
 
 declare i32 @__gxx_personality_v0(i32, i64, ptr, ptr)
 
 define { i8, i32 } @test2({ ptr, i32 } %x) {
 ; CHECK-LABEL: @test2(

llvm/test/Transforms/InstSimplify/insertvalue.ll

 ;
   %elem = extractvalue {i32, i32} %x, 0
   %v = insertvalue {i32, i32} poison, i32 %elem, 0
   ret {i32, i32} %v
 }
 
 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
   ret {i32, i32} %v
 }
 
 define {i32, i32} @insert_non_poison_into_undef({i32, i32} noundef %x) {
 ; CHECK-LABEL: @insert_non_poison_into_undef(
 ; CHECK-NEXT:    ret { i32, i32 } [[X:%.*]]
 ;
   %elem = extractvalue {i32, i32} %x, 0
   %v = insertvalue {i32, i32} undef, i32 %elem, 0
   ret {i32, i32} %v
 }