[PATCH] D141337: [SPIR-V] Fix translation of aggregate undef operands

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added inline comments.


================
Comment at: llvm/lib/Target/SPIRV/SPIRVEmitIntrinsics.cpp:179-180
         BuildCompositeIntrinsic(AggrC, Args);
+      } else if (isa<UndefValue>(Op) && Op->getType()->isAggregateType()) {
+        auto *AggrC = dyn_cast<Constant>(Op);
+        SmallVector<Value *> Args;
----------------
this dyn_cast to constant doesn't make much sense to me, either you don't need this second cast or you should dyn_cast to UndefValue?


================
Comment at: llvm/lib/Target/SPIRV/SPIRVEmitIntrinsics.cpp:185
+          Args.push_back(Elt);
+        BuildCompositeIntrinsic(AggrC, Args);
       } else if (auto *AggrC = dyn_cast<ConstantDataArray>(Op)) {
----------------
I'm guessing this doesn't handle nested structs properly


================
Comment at: llvm/test/CodeGen/SPIRV/instructions/undef-composite-store.ll:16
+; CHECK-NEXT: OpFunctionEnd
+
+define void @foo(ptr %ptr) {
----------------
Needs some tests with arrays, nested struct / array / vector combinations 


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D141337/new/

https://reviews.llvm.org/D141337