[SPIRV] Fix type mismatch assertion in insertvalue.

[maleadt]

The code was incorrectly converting all undef arguments to i32, while the spv_insertv intrinsics only expects that for the first operand, representing the aggregate type.

Fixes #127977

[llvmbot]

@llvm/pr-subscribers-backend-spir-v

Author: Tim Besard (maleadt)

Changes

The code was incorrectly converting all undef arguments to i32, while the spv_insertv intrinsics only expects that for the first operand, representing the aggregate type.

Fixes #127977

---

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

2 Files Affected:

• (modified) llvm/lib/Target/SPIRV/SPIRVEmitIntrinsics.cpp (+6-5)
• (added) llvm/test/CodeGen/SPIRV/instructions/insertvalue-undef-ptr.ll (+27)

diff --git a/llvm/lib/Target/SPIRV/SPIRVEmitIntrinsics.cpp b/llvm/lib/Target/SPIRV/SPIRVEmitIntrinsics.cpp
index cc95fde6a516d..8e467b5c700b0 100644
--- a/llvm/lib/Target/SPIRV/SPIRVEmitIntrinsics.cpp
+++ b/llvm/lib/Target/SPIRV/SPIRVEmitIntrinsics.cpp
@@ -1711,11 +1711,12 @@ Instruction *SPIRVEmitIntrinsics::visitInsertValueInst(InsertValueInst &I) {
   B.SetInsertPoint(&I);
   SmallVector<Type *, 1> Types = {I.getInsertedValueOperand()->getType()};
   SmallVector<Value *> Args;
-  for (auto &Op : I.operands())
-    if (isa<UndefValue>(Op))
-      Args.push_back(UndefValue::get(B.getInt32Ty()));
-    else
-      Args.push_back(Op);
+  Value *AggregateOp = I.getAggregateOperand();
+  if (isa<UndefValue>(AggregateOp))
+    Args.push_back(UndefValue::get(B.getInt32Ty()));
+  else
+    Args.push_back(AggregateOp);
+  Args.push_back(I.getInsertedValueOperand());
   for (auto &Op : I.indices())
     Args.push_back(B.getInt32(Op));
   Instruction *NewI =
diff --git a/llvm/test/CodeGen/SPIRV/instructions/insertvalue-undef-ptr.ll b/llvm/test/CodeGen/SPIRV/instructions/insertvalue-undef-ptr.ll
new file mode 100644
index 0000000000000..0676652ac1b39
--- /dev/null
+++ b/llvm/test/CodeGen/SPIRV/instructions/insertvalue-undef-ptr.ll
@@ -0,0 +1,27 @@
+; RUN: llc -verify-machineinstrs -O0 -mtriple=spirv64-unknown-unknown %s -o - | FileCheck %s
+
+; CHECK-LABEL: Begin function original_testcase
+define fastcc void @original_testcase() {
+top:
+  ; CHECK: OpCompositeInsert
+  %0 = insertvalue [1 x ptr] zeroinitializer, ptr poison, 0
+  ret void
+}
+
+; CHECK-LABEL: Begin function additional_testcases
+define fastcc void @additional_testcases() {
+top:
+  ; Test with different pointer types
+  ; CHECK: OpCompositeInsert
+  %1 = insertvalue [1 x ptr] zeroinitializer, ptr undef, 0
+  ; CHECK-NEXT: OpCompositeInsert
+  %2 = insertvalue {ptr, i32} zeroinitializer, ptr poison, 0
+  ; CHECK-NEXT: OpCompositeInsert
+  %3 = insertvalue {ptr, ptr} undef, ptr null, 0
+
+  ; Test with undef aggregate
+  ; CHECK-NEXT: OpCompositeInsert
+  %4 = insertvalue [1 x ptr] undef, ptr undef, 0
+
+  ret void
+}

[github-actions]

⚠️ undef deprecator found issues in your code. ⚠️

You can test this locally with the following command:

git diff -U0 --pickaxe-regex -S '([^a-zA-Z0-9#_-]undef[^a-zA-Z0-9_-]|UndefValue::get)' 'HEAD~1' HEAD llvm/test/CodeGen/SPIRV/instructions/insertvalue-undef-ptr.ll llvm/lib/Target/SPIRV/SPIRVEmitIntrinsics.cpp

The following files introduce new uses of undef:

• llvm/test/CodeGen/SPIRV/instructions/insertvalue-undef-ptr.ll

Undef is now deprecated and should only be used in the rare cases where no replacement is possible. For example, a load of uninitialized memory yields undef. You should use poison values for placeholders instead.

In tests, avoid using undef and having tests that trigger undefined behavior. If you need an operand with some unimportant value, you can add a new argument to the function and use that instead.

For example, this is considered a bad practice:

define void @fn() {
  ...
  br i1 undef, ...
}

Please use the following instead:

define void @fn(i1 %cond) {
  ...
  br i1 %cond, ...
}

Please refer to the Undefined Behavior Manual for more information.