[IR] Fix nested constant to instruction conversion

[wenju-he]

Fix two issues:

• If a constant is used in another constant, we need to insert newly
  created instructions to worklist so that constant used in them will
  be converted.
• Set debug info of original instruction to newly created instructions.

[llvmbot]

@llvm/pr-subscribers-llvm-ir

@llvm/pr-subscribers-llvm-transforms

Author: Wenju He (wenju-he)

Changes

[IR] Fix nested constant to instruction conversion

Fix two issues:

• If a constant is used in another constant, we need to insert newly
  created instructions to worklist so that constant used in them will
  be converted.
• Set debug info of original instruction to newly created instructions.

---

Full diff: https://github.com/llvm/llvm-project/pull/69682.diff

2 Files Affected:

• (modified) llvm/lib/IR/ReplaceConstant.cpp (+16-9)
• (modified) llvm/test/Transforms/LowerTypeTests/function-weak.ll (+13)

diff --git a/llvm/lib/IR/ReplaceConstant.cpp b/llvm/lib/IR/ReplaceConstant.cpp
index 58aa040eb032a87..7a98d9019b930b4 100644
--- a/llvm/lib/IR/ReplaceConstant.cpp
+++ b/llvm/lib/IR/ReplaceConstant.cpp
@@ -22,24 +22,29 @@ static bool isExpandableUser(User *U) {
   return isa<ConstantExpr>(U) || isa<ConstantAggregate>(U);
 }
 
-static Instruction *expandUser(Instruction *InsertPt, Constant *C) {
+static SmallVector<Instruction *, 4> expandUser(Instruction *InsertPt,
+                                                Constant *C) {
+  SmallVector<Instruction *, 4> NewInsts;
   if (auto *CE = dyn_cast<ConstantExpr>(C)) {
-    return CE->getAsInstruction(InsertPt);
+    NewInsts.push_back(CE->getAsInstruction(InsertPt));
   } else if (isa<ConstantStruct>(C) || isa<ConstantArray>(C)) {
     Value *V = PoisonValue::get(C->getType());
-    for (auto [Idx, Op] : enumerate(C->operands()))
+    for (auto [Idx, Op] : enumerate(C->operands())) {
       V = InsertValueInst::Create(V, Op, Idx, "", InsertPt);
-    return cast<Instruction>(V);
+      NewInsts.push_back(cast<Instruction>(V));
+    }
   } else if (isa<ConstantVector>(C)) {
     Type *IdxTy = Type::getInt32Ty(C->getContext());
     Value *V = PoisonValue::get(C->getType());
-    for (auto [Idx, Op] : enumerate(C->operands()))
+    for (auto [Idx, Op] : enumerate(C->operands())) {
       V = InsertElementInst::Create(V, Op, ConstantInt::get(IdxTy, Idx), "",
                                     InsertPt);
-    return cast<Instruction>(V);
+      NewInsts.push_back(cast<Instruction>(V));
+    }
   } else {
     llvm_unreachable("Not an expandable user");
   }
+  return NewInsts;
 }
 
 bool convertUsersOfConstantsToInstructions(ArrayRef<Constant *> Consts) {
@@ -85,9 +90,11 @@ bool convertUsersOfConstantsToInstructions(ArrayRef<Constant *> Consts) {
       if (auto *C = dyn_cast<Constant>(U.get())) {
         if (ExpandableUsers.contains(C)) {
           Changed = true;
-          Instruction *NI = expandUser(BI, C);
-          InstructionWorklist.insert(NI);
-          U.set(NI);
+          auto NewInsts = expandUser(BI, C);
+          for (auto *NI : NewInsts)
+            NI->setDebugLoc(BI->getDebugLoc());
+          InstructionWorklist.insert(NewInsts.begin(), NewInsts.end());
+          U.set(NewInsts.back());
         }
       }
     }
diff --git a/llvm/test/Transforms/LowerTypeTests/function-weak.ll b/llvm/test/Transforms/LowerTypeTests/function-weak.ll
index c765937f1991340..5f9041cd21b3c80 100644
--- a/llvm/test/Transforms/LowerTypeTests/function-weak.ll
+++ b/llvm/test/Transforms/LowerTypeTests/function-weak.ll
@@ -48,6 +48,19 @@ entry:
   ret void
 }
 
+define void @struct() {
+; CHECK-LABEL: define void @struct() {
+; CHECK: %0 = select i1 icmp ne (ptr @f, ptr null), ptr @.cfi.jumptable, ptr null
+; CHECK-NEXT: %1 = icmp ne ptr %0, null
+; CHECK-NEXT: %2 = insertvalue { i1, i8 } poison, i1 %1, 0
+; CHECK-NEXT: %3 = insertvalue { i1, i8 } %2, i8 0, 1
+; CHECK-NEXT: %x = extractvalue { i1, i8 } %3, 0
+
+entry:
+  %x = extractvalue { i1, i8 } { i1 icmp ne (ptr @f, ptr null), i8 0 }, 0
+  ret void
+}
+
 define void @phi(i1 %c) {
 ; CHECK-LABEL: define void @phi(i1 %c) {
 ; CHECK: entry:

[nikic]

For phi nodes this will take the debug loc from the terminator of the incoming block rather than the phi node -- is that intended?

[wenju-he]

Thank you for the review. I didn't notice this problem. I've updated to use debug loc from the original inst

[nikic]

LGTM