[llvm] [SPIR-V] Lower `select` instructions with aggregate operands (PR #201417)

Tim Besard via llvm-commits llvm-commits at lists.llvm.org
Mon Jun 8 09:40:11 PDT 2026


https://github.com/maleadt updated https://github.com/llvm/llvm-project/pull/201417

>From 88a5cf17bcadc61615fcbabe2235ad76ce2985ba Mon Sep 17 00:00:00 2001
From: Tim Besard <tim.besard at gmail.com>
Date: Wed, 3 Jun 2026 18:10:06 +0200
Subject: [PATCH 1/4] [SPIR-V] Lower select with composite constant operands

preprocessCompositeConstants() rewrites composite (array/struct) constant
operands into i32 value-ids produced by spv_const_composite. A SelectInst
derives its result type from its value operands, so this left the select with
a composite result type but i32 operands -- an invalid state the verifier
rejects with "Select values must have same type as select instruction" (and,
in an assertions build, trips the spv_extractv assertion in visitStoreInst).

PHINodes already have their aggregate result type mutated to the i32 value-id
type right after preprocessCompositeConstants(); do the same for a SelectInst
once both arms have been lowered to value-ids. Its extractvalue users are
lowered to spv_extractv by the existing visitor, so the result is a valid
OpSelect over the composite type.
---
 llvm/lib/Target/SPIRV/SPIRVEmitIntrinsics.cpp | 14 +++++++
 .../SPIRV/select-composite-constant.ll        | 41 +++++++++++++++++++
 2 files changed, 55 insertions(+)
 create mode 100644 llvm/test/CodeGen/SPIRV/select-composite-constant.ll

diff --git a/llvm/lib/Target/SPIRV/SPIRVEmitIntrinsics.cpp b/llvm/lib/Target/SPIRV/SPIRVEmitIntrinsics.cpp
index 180084a65d5b0..6d0c284b492e2 100644
--- a/llvm/lib/Target/SPIRV/SPIRVEmitIntrinsics.cpp
+++ b/llvm/lib/Target/SPIRV/SPIRVEmitIntrinsics.cpp
@@ -3539,6 +3539,20 @@ bool SPIRVEmitIntrinsics::runOnFunction(Function &Func) {
         Phi.mutateType(B.getInt32Ty());
       }
 
+  // A SelectInst, like a PHINode, takes its result type from its operands.
+  // preprocessCompositeConstants() rewrites composite constant arms to i32
+  // value-ids, so once both arms are lowered, mutate the select to match. Its
+  // original type is tracked in AggrConstTypes (used to assign the SPIR-V type)
+  // and its extractvalue users are lowered to spv_extractv below.
+  for (Instruction &I : instructions(Func))
+    if (auto *Sel = dyn_cast<SelectInst>(&I))
+      if (Sel->getType()->isAggregateType() &&
+          !Sel->getTrueValue()->getType()->isAggregateType() &&
+          !Sel->getFalseValue()->getType()->isAggregateType()) {
+        AggrConstTypes[Sel] = Sel->getType();
+        Sel->mutateType(B.getInt32Ty());
+      }
+
   preprocessBoolVectorBitcasts(Func);
   SmallVector<Instruction *> Worklist(
       llvm::make_pointer_range(instructions(Func)));
diff --git a/llvm/test/CodeGen/SPIRV/select-composite-constant.ll b/llvm/test/CodeGen/SPIRV/select-composite-constant.ll
new file mode 100644
index 0000000000000..ef51a339f5aeb
--- /dev/null
+++ b/llvm/test/CodeGen/SPIRV/select-composite-constant.ll
@@ -0,0 +1,41 @@
+; A `select` whose arms are composite (aggregate) constants used to crash in
+; SPIRVEmitIntrinsics: preprocessCompositeConstants() rewrites the composite
+; constant operands into i32 value-ids, which left the select with an aggregate
+; result type but i32 operands -- an invalid state rejected by the verifier with
+; "Select values must have same type as select instruction". Check that such a
+; select is now lowered to a valid OpSelect over the composite type.
+
+; RUN: llc -O0 -mtriple=spirv64-unknown-unknown %s -o - | FileCheck %s
+; RUN: %if spirv-tools %{ llc -O0 -mtriple=spirv64-unknown-unknown %s -o - -filetype=obj | spirv-val %}
+
+; CHECK-DAG: %[[#Float:]] = OpTypeFloat 32
+; CHECK-DAG: %[[#Int:]] = OpTypeInt 32 0
+; CHECK-DAG: %[[#Two:]] = OpConstant %[[#Int]] 2
+; CHECK-DAG: %[[#Array:]] = OpTypeArray %[[#Float]] %[[#Two]]
+; CHECK-DAG: %[[#Struct:]] = OpTypeStruct %[[#Float]] %[[#Float]]
+
+; The selects must be lowered to OpSelect over the composite type (not i32).
+; CHECK-DAG: %[[#ArrSel:]] = OpSelect %[[#Array]] %[[#]] %[[#]] %[[#]]
+; CHECK-DAG: %[[#StructSel:]] = OpSelect %[[#Struct]] %[[#]] %[[#]] %[[#]]
+; CHECK-DAG: OpCompositeExtract %[[#Float]] %[[#ArrSel]] 0
+; CHECK-DAG: OpCompositeExtract %[[#Float]] %[[#ArrSel]] 1
+; CHECK-DAG: OpCompositeExtract %[[#Float]] %[[#StructSel]] 0
+
+; Array-typed composite constant (e.g. a Julia Complex{Float32}).
+define spir_kernel void @select_array_constant(ptr addrspace(1) %out, i1 %c) {
+  %v = select i1 %c, [2 x float] [float 1.000000e+00, float 0.000000e+00], [2 x float] zeroinitializer
+  %e0 = extractvalue [2 x float] %v, 0
+  %e1 = extractvalue [2 x float] %v, 1
+  store float %e0, ptr addrspace(1) %out
+  %p1 = getelementptr float, ptr addrspace(1) %out, i64 1
+  store float %e1, ptr addrspace(1) %p1
+  ret void
+}
+
+; Struct-typed composite constant (e.g. a C _Complex float).
+define spir_kernel void @select_struct_constant(ptr addrspace(1) %out, i1 %c) {
+  %v = select i1 %c, { float, float } { float 1.000000e+00, float 2.000000e+00 }, { float, float } zeroinitializer
+  %e0 = extractvalue { float, float } %v, 0
+  store float %e0, ptr addrspace(1) %out
+  ret void
+}

>From 6c1fff8aee00329bd1c9d416682486ef7459bd28 Mon Sep 17 00:00:00 2001
From: Tim Besard <tim.besard at gmail.com>
Date: Wed, 3 Jun 2026 18:50:34 +0200
Subject: [PATCH 2/4] [SPIR-V] Lower select with non-constant aggregate
 operands

The previous change handled selects whose aggregate arms are both composite
constants (rewritten to i32 value-ids by preprocessCompositeConstants). But a
select arm can also be a non-constant aggregate -- a load, insertvalue, call
result, or another select -- which is lowered to a value-id later, during the
visitor pass. Such selects (and the all-non-constant case) still crashed: a
SelectInst was not a recognized consumer of aggregate value-ids and hit
"illegal aggregate intrinsic user" in replaceMemInstrUses().

Generalize the handling to mirror PHINode, which already supports this:

 - Mutate every aggregate-typed select to the i32 value-id type early, not just
   those whose arms are already lowered, exactly as the PHI loop does.
 - Recognize SelectInst alongside PHINode in replaceMemInstrUses(), so that when
   an aggregate operand is rewritten to its value-id the select's operand is
   updated and its extractvalue users are lowered to spv_extractv. The shared
   extractvalue-lowering logic is factored out into lowerExtractvUsers().

The original type is tracked in AggrConstTypes, so the selector still emits a
valid OpSelect over the composite type.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply at anthropic.com>
---
 llvm/lib/Target/SPIRV/SPIRVEmitIntrinsics.cpp | 69 +++++++++-------
 llvm/test/CodeGen/SPIRV/select-aggregate.ll   | 81 +++++++++++++++++++
 2 files changed, 122 insertions(+), 28 deletions(-)
 create mode 100644 llvm/test/CodeGen/SPIRV/select-aggregate.ll

diff --git a/llvm/lib/Target/SPIRV/SPIRVEmitIntrinsics.cpp b/llvm/lib/Target/SPIRV/SPIRVEmitIntrinsics.cpp
index 6d0c284b492e2..cbd6bb2195282 100644
--- a/llvm/lib/Target/SPIRV/SPIRVEmitIntrinsics.cpp
+++ b/llvm/lib/Target/SPIRV/SPIRVEmitIntrinsics.cpp
@@ -267,6 +267,7 @@ class SPIRVEmitIntrinsics
                         bool IsPostprocessing);
 
   void replaceMemInstrUses(Instruction *Old, Instruction *New, IRBuilder<> &B);
+  void lowerExtractvUsers(Value *Aggr, IRBuilder<> &B);
   void processInstrAfterVisit(Instruction *I, IRBuilder<> &B);
   bool insertAssignPtrTypeIntrs(Instruction *I, IRBuilder<> &B,
                                 bool UnknownElemTypeI8);
@@ -1578,28 +1579,20 @@ void SPIRVEmitIntrinsics::replaceMemInstrUses(Instruction *Old,
           CI->setCalledFunction(NewF);
         }
       }
-    } else if (auto *Phi = dyn_cast<PHINode>(U)) {
-      if (Phi->getType() != New->getType()) {
-        Phi->mutateType(New->getType());
-        Phi->replaceUsesOfWith(Old, New);
-        // Convert extractvalue users of the mutated PHI to spv_extractv
-        SmallVector<ExtractValueInst *, 4> EVUsers;
-        for (User *PhiUser : Phi->users())
-          if (auto *EV = dyn_cast<ExtractValueInst>(PhiUser))
-            EVUsers.push_back(EV);
-        for (ExtractValueInst *EV : EVUsers) {
-          B.SetInsertPoint(EV);
-          SmallVector<Value *> Args(EV->operand_values());
-          for (unsigned Idx : EV->indices())
-            Args.push_back(B.getInt32(Idx));
-          auto *NewEV =
-              B.CreateIntrinsic(Intrinsic::spv_extractv, {EV->getType()}, Args);
-          EV->replaceAllUsesWith(NewEV);
-          DeletedInstrs.insert(EV);
-          EV->eraseFromParent();
-        }
+    } else if (isa<PHINode>(U) || isa<SelectInst>(U)) {
+      // A PHINode or SelectInst derives its result type from its operands, so
+      // replacing an aggregate operand with its i32 value-id may require
+      // mutating the user to match. Its extractvalue users are then lowered to
+      // spv_extractv. (When the user's type was already settled to i32 by the
+      // early mutation pass, the types match and its extractvalue users are
+      // lowered later by visitExtractValueInst instead.)
+      auto *UI = cast<Instruction>(U);
+      if (UI->getType() != New->getType()) {
+        UI->mutateType(New->getType());
+        UI->replaceUsesOfWith(Old, New);
+        lowerExtractvUsers(UI, B);
       } else {
-        Phi->replaceUsesOfWith(Old, New);
+        UI->replaceUsesOfWith(Old, New);
       }
     } else {
       llvm_unreachable("illegal aggregate intrinsic user");
@@ -1609,6 +1602,27 @@ void SPIRVEmitIntrinsics::replaceMemInstrUses(Instruction *Old,
   Old->eraseFromParent();
 }
 
+// Lower extractvalue users of an aggregate value that has been rewritten to an
+// i32 value-id (e.g. a PHINode or SelectInst whose type was mutated) into
+// spv_extractv intrinsics.
+void SPIRVEmitIntrinsics::lowerExtractvUsers(Value *Aggr, IRBuilder<> &B) {
+  SmallVector<ExtractValueInst *, 4> EVUsers;
+  for (User *U : Aggr->users())
+    if (auto *EV = dyn_cast<ExtractValueInst>(U))
+      EVUsers.push_back(EV);
+  for (ExtractValueInst *EV : EVUsers) {
+    B.SetInsertPoint(EV);
+    SmallVector<Value *> Args(EV->operand_values());
+    for (unsigned Idx : EV->indices())
+      Args.push_back(B.getInt32(Idx));
+    auto *NewEV =
+        B.CreateIntrinsic(Intrinsic::spv_extractv, {EV->getType()}, Args);
+    EV->replaceAllUsesWith(NewEV);
+    DeletedInstrs.insert(EV);
+    EV->eraseFromParent();
+  }
+}
+
 void SPIRVEmitIntrinsics::preprocessUndefs(IRBuilder<> &B) {
   const SPIRVSubtarget *STI = TM.getSubtargetImpl(*CurrF);
   bool HasPoisonExt =
@@ -3540,15 +3554,14 @@ bool SPIRVEmitIntrinsics::runOnFunction(Function &Func) {
       }
 
   // A SelectInst, like a PHINode, takes its result type from its operands.
-  // preprocessCompositeConstants() rewrites composite constant arms to i32
-  // value-ids, so once both arms are lowered, mutate the select to match. Its
-  // original type is tracked in AggrConstTypes (used to assign the SPIR-V type)
-  // and its extractvalue users are lowered to spv_extractv below.
+  // Aggregate arms are lowered to i32 value-ids (composite constants here,
+  // loads and other producers during the visitor pass below), so mutate an
+  // aggregate select to match, just as the PHI loop above does. Its original
+  // type is tracked in AggrConstTypes (used to assign the SPIR-V type) and its
+  // extractvalue users are lowered to spv_extractv.
   for (Instruction &I : instructions(Func))
     if (auto *Sel = dyn_cast<SelectInst>(&I))
-      if (Sel->getType()->isAggregateType() &&
-          !Sel->getTrueValue()->getType()->isAggregateType() &&
-          !Sel->getFalseValue()->getType()->isAggregateType()) {
+      if (Sel->getType()->isAggregateType()) {
         AggrConstTypes[Sel] = Sel->getType();
         Sel->mutateType(B.getInt32Ty());
       }
diff --git a/llvm/test/CodeGen/SPIRV/select-aggregate.ll b/llvm/test/CodeGen/SPIRV/select-aggregate.ll
new file mode 100644
index 0000000000000..5771d6dfadfa7
--- /dev/null
+++ b/llvm/test/CodeGen/SPIRV/select-aggregate.ll
@@ -0,0 +1,81 @@
+; A `select` between aggregate (array/struct) values whose arms are not both
+; composite constants -- one or both arms are loaded, or are themselves a
+; select -- used to crash in SPIRVEmitIntrinsics ("illegal aggregate intrinsic
+; user", or the verifier's "Select values must have same type as select
+; instruction"). Check that such selects are lowered to a valid OpSelect over
+; the composite type. The all-constant case is covered by
+; select-composite-constant.ll.
+
+; RUN: llc -O0 -mtriple=spirv64-unknown-unknown %s -o - | FileCheck %s
+; RUN: %if spirv-tools %{ llc -O0 -mtriple=spirv64-unknown-unknown %s -o - -filetype=obj | spirv-val %}
+
+; CHECK-DAG: %[[#Float:]] = OpTypeFloat 32
+; CHECK-DAG: %[[#Int:]] = OpTypeInt 32 0
+; CHECK-DAG: %[[#Two:]] = OpConstant %[[#Int]] 2
+; CHECK-DAG: %[[#Array:]] = OpTypeArray %[[#Float]] %[[#Two]]
+; CHECK-DAG: %[[#Struct:]] = OpTypeStruct %[[#Float]] %[[#Float]]
+
+; Both arms are loaded (non-constant) aggregates.
+; CHECK: %[[#A:]] = OpLoad %[[#Array]]
+; CHECK: %[[#B:]] = OpLoad %[[#Array]]
+; CHECK: %[[#Sel0:]] = OpSelect %[[#Array]] %[[#]] %[[#A]] %[[#B]]
+; CHECK: OpCompositeExtract %[[#Float]] %[[#Sel0]] 0
+; CHECK: OpCompositeExtract %[[#Float]] %[[#Sel0]] 1
+define spir_kernel void @both_loaded(ptr addrspace(1) %out, ptr addrspace(1) %pa, ptr addrspace(1) %pb, i1 %c) {
+  %a = load [2 x float], ptr addrspace(1) %pa
+  %b = load [2 x float], ptr addrspace(1) %pb
+  %v = select i1 %c, [2 x float] %a, [2 x float] %b
+  %e0 = extractvalue [2 x float] %v, 0
+  %e1 = extractvalue [2 x float] %v, 1
+  store float %e0, ptr addrspace(1) %out
+  %p1 = getelementptr float, ptr addrspace(1) %out, i64 1
+  store float %e1, ptr addrspace(1) %p1
+  ret void
+}
+
+; Mixed: one arm loaded, the other a composite constant.
+; CHECK: %[[#M:]] = OpLoad %[[#Array]]
+; CHECK: %[[#Sel1:]] = OpSelect %[[#Array]] %[[#]] %[[#M]] %[[#]]
+; CHECK: OpCompositeExtract %[[#Float]] %[[#Sel1]] 0
+define spir_kernel void @mixed_array(ptr addrspace(1) %out, ptr addrspace(1) %pa, i1 %c) {
+  %a = load [2 x float], ptr addrspace(1) %pa
+  %v = select i1 %c, [2 x float] %a, [2 x float] [float 1.000000e+00, float 0.000000e+00]
+  %e0 = extractvalue [2 x float] %v, 0
+  store float %e0, ptr addrspace(1) %out
+  ret void
+}
+
+; Mixed struct-typed select.
+; CHECK: %[[#S:]] = OpLoad %[[#Struct]]
+; CHECK: %[[#Sel2:]] = OpSelect %[[#Struct]] %[[#]] %[[#S]] %[[#]]
+; CHECK: OpCompositeExtract %[[#Float]] %[[#Sel2]] 0
+define spir_kernel void @mixed_struct(ptr addrspace(1) %out, ptr addrspace(1) %pa, i1 %c) {
+  %a = load { float, float }, ptr addrspace(1) %pa
+  %v = select i1 %c, { float, float } %a, { float, float } { float 1.000000e+00, float 2.000000e+00 }
+  %e0 = extractvalue { float, float } %v, 0
+  store float %e0, ptr addrspace(1) %out
+  ret void
+}
+
+; A select whose arm is itself a select.
+; CHECK: %[[#Inner:]] = OpSelect %[[#Array]] %[[#]] %[[#]] %[[#]]
+; CHECK: %[[#Outer:]] = OpSelect %[[#Array]] %[[#]] %[[#Inner]] %[[#]]
+; CHECK: OpCompositeExtract %[[#Float]] %[[#Outer]] 0
+define spir_kernel void @nested(ptr addrspace(1) %out, ptr addrspace(1) %pa, i1 %c, i1 %d) {
+  %a = load [2 x float], ptr addrspace(1) %pa
+  %inner = select i1 %d, [2 x float] %a, [2 x float] zeroinitializer
+  %v = select i1 %c, [2 x float] %inner, [2 x float] [float 1.000000e+00, float 0.000000e+00]
+  %e0 = extractvalue [2 x float] %v, 0
+  store float %e0, ptr addrspace(1) %out
+  ret void
+}
+
+; The aggregate select result is stored directly, without an extractvalue.
+; CHECK: %[[#Sel3:]] = OpSelect %[[#Array]] %[[#]] %[[#]] %[[#]]
+; CHECK: OpStore %[[#]] %[[#Sel3]]
+define spir_kernel void @store_direct(ptr addrspace(1) %out, ptr addrspace(1) %pa, i1 %c) {
+  %a = load [2 x float], ptr addrspace(1) %pa
+  %v = select i1 %c, [2 x float] %a, [2 x float] zeroinitializer
+  store [2 x float] %v, ptr addrspace(1) %out
+  ret void
+}

>From 48af1b4fd71bb41663a43dbbf956c2d8ade8c1df Mon Sep 17 00:00:00 2001
From: Tim Besard <tim.besard at gmail.com>
Date: Fri, 5 Jun 2026 10:15:48 +0200
Subject: [PATCH 3/4] [SPIR-V] Remove unreachable type mutation in
 replaceMemInstrUses

Aggregate PHIs and selects are mutated to the i32 value-id type up
front in runOnFunction, before the visitor pass runs, so the types
always match by the time replaceMemInstrUses sees them. This path has
been unreachable since its introduction in #186086, which added the
up-front PHI mutation in the same commit. Replace it with an assert,
and drop DeletedInstrs, which was only populated on this path.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply at anthropic.com>
---
 llvm/lib/Target/SPIRV/SPIRVEmitIntrinsics.cpp | 47 +++----------------
 1 file changed, 7 insertions(+), 40 deletions(-)

diff --git a/llvm/lib/Target/SPIRV/SPIRVEmitIntrinsics.cpp b/llvm/lib/Target/SPIRV/SPIRVEmitIntrinsics.cpp
index cbd6bb2195282..221b0712fe17d 100644
--- a/llvm/lib/Target/SPIRV/SPIRVEmitIntrinsics.cpp
+++ b/llvm/lib/Target/SPIRV/SPIRVEmitIntrinsics.cpp
@@ -190,7 +190,6 @@ class SPIRVEmitIntrinsics
   DenseMap<Instruction *, Constant *> AggrConsts;
   DenseMap<Instruction *, Type *> AggrConstTypes;
   DenseSet<Instruction *> AggrStores;
-  SmallPtrSet<Instruction *, 8> DeletedInstrs;
   GlobalVariableUsers GVUsers;
   std::unordered_set<Value *> Named;
 
@@ -267,7 +266,6 @@ class SPIRVEmitIntrinsics
                         bool IsPostprocessing);
 
   void replaceMemInstrUses(Instruction *Old, Instruction *New, IRBuilder<> &B);
-  void lowerExtractvUsers(Value *Aggr, IRBuilder<> &B);
   void processInstrAfterVisit(Instruction *I, IRBuilder<> &B);
   bool insertAssignPtrTypeIntrs(Instruction *I, IRBuilder<> &B,
                                 bool UnknownElemTypeI8);
@@ -1580,20 +1578,13 @@ void SPIRVEmitIntrinsics::replaceMemInstrUses(Instruction *Old,
         }
       }
     } else if (isa<PHINode>(U) || isa<SelectInst>(U)) {
-      // A PHINode or SelectInst derives its result type from its operands, so
-      // replacing an aggregate operand with its i32 value-id may require
-      // mutating the user to match. Its extractvalue users are then lowered to
-      // spv_extractv. (When the user's type was already settled to i32 by the
-      // early mutation pass, the types match and its extractvalue users are
-      // lowered later by visitExtractValueInst instead.)
-      auto *UI = cast<Instruction>(U);
-      if (UI->getType() != New->getType()) {
-        UI->mutateType(New->getType());
-        UI->replaceUsesOfWith(Old, New);
-        lowerExtractvUsers(UI, B);
-      } else {
-        UI->replaceUsesOfWith(Old, New);
-      }
+      // Aggregate-typed PHIs and selects have already been mutated to the
+      // i32 value-id type up front in runOnFunction, so only the operand
+      // needs replacing here; their extractvalue users are lowered to
+      // spv_extractv by visitExtractValueInst.
+      assert(U->getType() == New->getType() &&
+             "aggregate PHI/select should have been mutated to value-id type");
+      U->replaceUsesOfWith(Old, New);
     } else {
       llvm_unreachable("illegal aggregate intrinsic user");
     }
@@ -1602,27 +1593,6 @@ void SPIRVEmitIntrinsics::replaceMemInstrUses(Instruction *Old,
   Old->eraseFromParent();
 }
 
-// Lower extractvalue users of an aggregate value that has been rewritten to an
-// i32 value-id (e.g. a PHINode or SelectInst whose type was mutated) into
-// spv_extractv intrinsics.
-void SPIRVEmitIntrinsics::lowerExtractvUsers(Value *Aggr, IRBuilder<> &B) {
-  SmallVector<ExtractValueInst *, 4> EVUsers;
-  for (User *U : Aggr->users())
-    if (auto *EV = dyn_cast<ExtractValueInst>(U))
-      EVUsers.push_back(EV);
-  for (ExtractValueInst *EV : EVUsers) {
-    B.SetInsertPoint(EV);
-    SmallVector<Value *> Args(EV->operand_values());
-    for (unsigned Idx : EV->indices())
-      Args.push_back(B.getInt32(Idx));
-    auto *NewEV =
-        B.CreateIntrinsic(Intrinsic::spv_extractv, {EV->getType()}, Args);
-    EV->replaceAllUsesWith(NewEV);
-    DeletedInstrs.insert(EV);
-    EV->eraseFromParent();
-  }
-}
-
 void SPIRVEmitIntrinsics::preprocessUndefs(IRBuilder<> &B) {
   const SPIRVSubtarget *STI = TM.getSubtargetImpl(*CurrF);
   bool HasPoisonExt =
@@ -3490,7 +3460,6 @@ bool SPIRVEmitIntrinsics::runOnFunction(Function &Func) {
   AggrConsts.clear();
   AggrConstTypes.clear();
   AggrStores.clear();
-  DeletedInstrs.clear();
 
   processParamTypesByFunHeader(CurrF, B);
 
@@ -3606,8 +3575,6 @@ bool SPIRVEmitIntrinsics::runOnFunction(Function &Func) {
         deduceOperandElementType(&Phi, nullptr);
 
   for (auto *I : Worklist) {
-    if (DeletedInstrs.count(I))
-      continue;
     TrackConstants = true;
     if (!I->getType()->isVoidTy() || isa<StoreInst>(I))
       setInsertPointAfterDef(B, I);

>From d90fb90b42c97bb3e192fef99f265baf306fa5cc Mon Sep 17 00:00:00 2001
From: Tim Besard <tim.besard at gmail.com>
Date: Mon, 8 Jun 2026 18:37:49 +0200
Subject: [PATCH 4/4] [SPIR-V] Merge aggregate PHI and select type-mutation
 loops

PHINode and SelectInst both take their result type from their operands and
need the same up-front mutation to the i32 value-id type. Fold the two loops
into one pass over the instructions, as suggested in review.

NFC.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply at anthropic.com>
---
 llvm/lib/Target/SPIRV/SPIRVEmitIntrinsics.cpp | 34 ++++++++-----------
 1 file changed, 15 insertions(+), 19 deletions(-)

diff --git a/llvm/lib/Target/SPIRV/SPIRVEmitIntrinsics.cpp b/llvm/lib/Target/SPIRV/SPIRVEmitIntrinsics.cpp
index 221b0712fe17d..835f8894ed289 100644
--- a/llvm/lib/Target/SPIRV/SPIRVEmitIntrinsics.cpp
+++ b/llvm/lib/Target/SPIRV/SPIRVEmitIntrinsics.cpp
@@ -3515,25 +3515,21 @@ bool SPIRVEmitIntrinsics::runOnFunction(Function &Func) {
   simplifyNullAddrSpaceCasts();
   preprocessCompositeConstants(B);
 
-  for (BasicBlock &BB : Func)
-    for (PHINode &Phi : BB.phis())
-      if (Phi.getType()->isAggregateType()) {
-        AggrConstTypes[&Phi] = Phi.getType();
-        Phi.mutateType(B.getInt32Ty());
-      }
-
-  // A SelectInst, like a PHINode, takes its result type from its operands.
-  // Aggregate arms are lowered to i32 value-ids (composite constants here,
-  // loads and other producers during the visitor pass below), so mutate an
-  // aggregate select to match, just as the PHI loop above does. Its original
-  // type is tracked in AggrConstTypes (used to assign the SPIR-V type) and its
-  // extractvalue users are lowered to spv_extractv.
-  for (Instruction &I : instructions(Func))
-    if (auto *Sel = dyn_cast<SelectInst>(&I))
-      if (Sel->getType()->isAggregateType()) {
-        AggrConstTypes[Sel] = Sel->getType();
-        Sel->mutateType(B.getInt32Ty());
-      }
+  // A PHINode or SelectInst takes its result type from its operands. Aggregate
+  // arms are lowered to i32 value-ids (composite constants here, loads and other
+  // producers during the visitor pass below), so mutate an aggregate PHI or
+  // select to match. The original type is tracked in AggrConstTypes (used to
+  // assign the SPIR-V type) and its extractvalue users are lowered to
+  // spv_extractv.
+  Type *I32Ty = B.getInt32Ty();
+  for (Instruction &I : instructions(Func)) {
+    if (!isa<PHINode>(I) && !isa<SelectInst>(I))
+      continue;
+    if (!I.getType()->isAggregateType())
+      continue;
+    AggrConstTypes[&I] = I.getType();
+    I.mutateType(I32Ty);
+  }
 
   preprocessBoolVectorBitcasts(Func);
   SmallVector<Instruction *> Worklist(



More information about the llvm-commits mailing list