[llvm] [SPIR-V] Lower nested aggregate insertvalue operands (PR #204239)

[Tim Besard via llvm-commits]

================
@@ -2465,6 +2465,17 @@ SPIRVEmitIntrinsicsImpl::visitExtractValueInst(ExtractValueInst &I) {
     Args.push_back(B.getInt32(Op));
   Instruction *NewI = B.CreateIntrinsicWithoutFolding(Intrinsic::spv_extractv,
                                                       {I.getType()}, {Args});
+  // If this aggregate extract feeds another insertvalue, the extracted
+  // composite is used as a SPIR-V value-id by llvm.spv.insertv. Keep the real
+  // aggregate type in metadata, but expose the value itself as i32 so the
+  // intrinsic signature remains valid.
+  if (NewI->getType()->isAggregateType() &&
+      any_of(I.users(), [](User *U) { return isa<InsertValueInst>(U); })) {
----------------
maleadt wrote:

Ah, I hadn't considered already-lowered intrinsics. I'm not sure simply adding `spv_insertv` here works though, since AFAIU the signature of that intrinsic would then need to be changed as well. LLM-generated example:

> Suppose the aggregate extract is the value being inserted:
> 
> ```llvm
> %e = extractvalue [1 x [1 x i64]] %outer, 0
> %i = insertvalue [1 x [1 x i64]] %base, [1 x i64] %e, 0
> ```
> 
> If the extract is lowered first, it becomes an `i32` SPIR-V value ID, and the subsequent insertion is correctly overloaded for `i32`:
> 
> ```llvm
> %e = call i32 @llvm.spv.extractv.i32(i32 %outer, i32 0)
> 
> %i = call i32 (i32, i32, ...)
>     @llvm.spv.insertv.i32(i32 %base, i32 %e, i32 0)
> ```
> 
> But if the insertion has already been lowered while `%e` still has its aggregate type, its declaration is overloaded for `[1 x i64]`:
> 
> ```llvm
> %e = extractvalue [1 x [1 x i64]] %outer, 0
> 
> %i = call i32 (i32, [1 x i64], ...)
>     @llvm.spv.insertv.a1i64(
>         i32 %base, [1 x i64] %e, i32 0)
> ```
> 
> Now `visitExtractValueInst` notices the `spv_insertv` user and mutates `%e` to `i32`. Merely replacing the operand would produce:
> 
> ```llvm
> %e = call i32 @llvm.spv.extractv.i32(i32 %outer, i32 0)
> 
> ; Invalid: the declaration expects [1 x i64], but %e is now i32.
> %i = call i32 (i32, [1 x i64], ...)
>     @llvm.spv.insertv.a1i64(
>         i32 %base, i32 %e, i32 0)
> ```
> 
> The call would also need to be retargeted:
> 
> ```llvm
> %i = call i32 (i32, i32, ...)
>     @llvm.spv.insertv.i32(
>         i32 %base, i32 %e, i32 0)
> ```

Maybe we should make it so that aggregate conversion happens before visiting insertions?

https://github.com/llvm/llvm-project/pull/204239