[SPIR-V] Emit spv_undef intrinsic for aggregate undef operands

[mpaszkowski]

Summary

This change adds a new spv_undef intrinsic which is emitted in place of aggregate undef operands and later selected to single OpUndef SPIR-V instruction. The behavior matches that of Khronos SPIR-V Translator and should support nested aggregates.

This is an alternative better approach to the one discussed in https://reviews.llvm.org/D141337.

[mpaszkowski]

Fixed formatting

[iliya-diyachkov]

llvm/lib/Target/SPIRV/SPIRVEmitIntrinsics.cpp

165

The indentation level could be reduced by

if (!condition)
  continue;

[arsenm]

llvm/lib/Target/SPIRV/SPIRVEmitIntrinsics.cpp

155

I don't really understand why you need to handle this in an IR pass, undoes could easily appear at any time

166

You're using isa and dyn_cast, just use dyn_cast once

llvm/lib/Target/SPIRV/SPIRVInstructionSelector.cpp

1345

dead break

[iliya-diyachkov]

llvm/lib/Target/SPIRV/SPIRVEmitIntrinsics.cpp

155

@arsenm This is required just before IRTranslator to pass aggregate undefs through it. Undefs appearing in other places are already supported.

[mpaszkowski]

Fixed issues

[iliya-diyachkov]

The patch looks good to me, thanks Michal.

llvm/include/llvm/IR/IntrinsicsSPIRV.td

let TargetPrefix = "spv" in {
   def int_spv_extractelt : Intrinsic<[llvm_any_ty], [llvm_any_ty, llvm_anyint_ty]>;
   def int_spv_insertelt : Intrinsic<[llvm_any_ty], [llvm_any_ty, llvm_any_ty, llvm_anyint_ty]>;
   def int_spv_const_composite : Intrinsic<[llvm_i32_ty], [llvm_vararg_ty]>;
   def int_spv_bitcast : Intrinsic<[llvm_any_ty], [llvm_any_ty]>;
   def int_spv_switch : Intrinsic<[], [llvm_any_ty, llvm_vararg_ty]>;
   def int_spv_cmpxchg : Intrinsic<[llvm_i32_ty], [llvm_any_ty, llvm_vararg_ty]>;
   def int_spv_unreachable : Intrinsic<[], []>;
   def int_spv_alloca : Intrinsic<[llvm_any_ty], []>;
+  def int_spv_undef : Intrinsic<[llvm_i32_ty], []>;
 }

llvm/lib/Target/SPIRV/SPIRVEmitIntrinsics.cpp

: public FunctionPass,
       public InstVisitor<SPIRVEmitIntrinsics, Instruction *> {
   SPIRVTargetMachine *TM = nullptr;
   IRBuilder<> *IRB = nullptr;
   Function *F = nullptr;
   bool TrackConstants = true;
   DenseMap<Instruction *, Constant *> AggrConsts;
   DenseSet<Instruction *> AggrStores;
   void preprocessCompositeConstants();
+  void preprocessUndefs();
   CallInst *buildIntrWithMD(Intrinsic::ID IntrID, ArrayRef<Type *> Types,
                             Value *Arg, Value *Arg2) {
     ConstantAsMetadata *CM = ValueAsMetadata::getConstant(Arg);
     MDTuple *TyMD = MDNode::get(F->getContext(), CM);
     MetadataAsValue *VMD = MetadataAsValue::get(F->getContext(), TyMD);
     return IRB->CreateIntrinsic(IntrID, {Types}, {Arg2, VMD});
   }
   void replaceMemInstrUses(Instruction *Old, Instruction *New);
if (isAssignTypeInstr(U)) {
       U->replaceUsesOfWith(Old, New);
     } else {
       llvm_unreachable("illegal aggregate intrinsic user");
     }
   }
   Old->eraseFromParent();
 }
 
+void SPIRVEmitIntrinsics::preprocessUndefs() {
+  std::queue<Instruction *> Worklist;
+  for (auto &I : instructions(F))
+    Worklist.push(&I);
+
+  while (!Worklist.empty()) {
+    Instruction *I = Worklist.front();
+    Worklist.pop();
+
+    for (auto &Op : I->operands()) {
+      auto *AggrUndef = dyn_cast<UndefValue>(Op);
+      if (!AggrUndef || !Op->getType()->isAggregateType())
+        continue;
+
+      IRB->SetInsertPoint(I);
+      auto *IntrUndef = IRB->CreateIntrinsic(Intrinsic::spv_undef, {}, {});
+      Worklist.push(IntrUndef);
+      I->replaceUsesOfWith(Op, IntrUndef);
+      AggrConsts[IntrUndef] = AggrUndef;
+    }
+  }
+}
+
 void SPIRVEmitIntrinsics::preprocessCompositeConstants() {
   std::queue<Instruction *> Worklist;
   for (auto &I : instructions(F))
     Worklist.push(&I);
 
   while (!Worklist.empty()) {
     auto *I = Worklist.front();
     assert(I);

 }
 
 void SPIRVEmitIntrinsics::insertAssignTypeIntrs(Instruction *I) {
   Type *Ty = I->getType();
   if (!Ty->isVoidTy() && requireAssignType(I)) {
     setInsertPointSkippingPhis(*IRB, I->getNextNode());
     Type *TypeToAssign = Ty;
     if (auto *II = dyn_cast<IntrinsicInst>(I)) {
-      if (II->getIntrinsicID() == Intrinsic::spv_const_composite) {
+      if (II->getIntrinsicID() == Intrinsic::spv_const_composite ||
+          II->getIntrinsicID() == Intrinsic::spv_undef) {
         auto t = AggrConsts.find(II);
         assert(t != AggrConsts.end());
         TypeToAssign = t->second->getType();
       }
     }
     Constant *Const = Constant::getNullValue(TypeToAssign);
     buildIntrWithMD(Intrinsic::spv_assign_type, {Ty}, Const, I);
   }
if (ElTy->isAggregateType() || ElTy->isVectorTy() ||
         !PTy->isOpaqueOrPointeeTypeMatches(ElTy))
       AggrStores.insert(&I);
   }
 
   IRB->SetInsertPoint(&Func.getEntryBlock().front());
   for (auto &GV : Func.getParent()->globals())
     processGlobalValue(GV);
 
+  preprocessUndefs();
   preprocessCompositeConstants();
   SmallVector<Instruction *> Worklist;
   for (auto &I : instructions(Func))
     Worklist.push_back(&I);
 
   for (auto &I : Worklist)
     insertAssignTypeIntrs(I);
 

llvm/lib/Target/SPIRV/SPIRVInstructionSelector.cpp

 
 bool SPIRVInstructionSelector::selectIntrinsic(Register ResVReg,
                                                const SPIRVType *ResType,
                                                MachineInstr &I) const {
   MachineBasicBlock &BB = *I.getParent();
   switch (I.getIntrinsicID()) {
   case Intrinsic::spv_load:
     return selectLoad(ResVReg, ResType, I);
-    break;
   case Intrinsic::spv_store:
     return selectStore(I);
-    break;
   case Intrinsic::spv_extractv:
     return selectExtractVal(ResVReg, ResType, I);
-    break;
   case Intrinsic::spv_insertv:
     return selectInsertVal(ResVReg, ResType, I);
-    break;
   case Intrinsic::spv_extractelt:
     return selectExtractElt(ResVReg, ResType, I);
-    break;
   case Intrinsic::spv_insertelt:
     return selectInsertElt(ResVReg, ResType, I);
-    break;
   case Intrinsic::spv_gep:
     return selectGEP(ResVReg, ResType, I);
-    break;
   case Intrinsic::spv_unref_global:
   case Intrinsic::spv_init_global: {
     MachineInstr *MI = MRI->getVRegDef(I.getOperand(1).getReg());
     MachineInstr *Init = I.getNumExplicitOperands() > 2
                              ? MRI->getVRegDef(I.getOperand(2).getReg())
                              : nullptr;
     assert(MI);
     return selectGlobalValue(MI->getOperand(0).getReg(), *MI, Init);
-  } break;
+  }
+  case Intrinsic::spv_undef: {
+    auto MIB = BuildMI(BB, I, I.getDebugLoc(), TII.get(SPIRV::OpUndef))
+                   .addDef(ResVReg)
+                   .addUse(GR.getSPIRVTypeID(ResType));
+    return MIB.constrainAllUses(TII, TRI, RBI);
+  }
   case Intrinsic::spv_const_composite: {
     // If no values are attached, the composite is null constant.
     bool IsNull = I.getNumExplicitDefs() + 1 == I.getNumExplicitOperands();
     unsigned Opcode =
         IsNull ? SPIRV::OpConstantNull : SPIRV::OpConstantComposite;
     auto MIB = BuildMI(BB, I, I.getDebugLoc(), TII.get(Opcode))
                    .addDef(ResVReg)
                    .addUse(GR.getSPIRVTypeID(ResType));
     // skip type MD node we already used when generated assign.type for this
     if (!IsNull) {
       for (unsigned i = I.getNumExplicitDefs() + 1;
            i < I.getNumExplicitOperands(); ++i) {
         MIB.addUse(I.getOperand(i).getReg());
       }
     }
     return MIB.constrainAllUses(TII, TRI, RBI);
-  } break;
+  }
   case Intrinsic::spv_assign_name: {
     auto MIB = BuildMI(BB, I, I.getDebugLoc(), TII.get(SPIRV::OpName));
     MIB.addUse(I.getOperand(I.getNumExplicitDefs() + 1).getReg());
     for (unsigned i = I.getNumExplicitDefs() + 2;
          i < I.getNumExplicitOperands(); ++i) {
       MIB.addImm(I.getOperand(i).getImm());
     }
     return MIB.constrainAllUses(TII, TRI, RBI);
-  } break;
+  }
   case Intrinsic::spv_switch: {
     auto MIB = BuildMI(BB, I, I.getDebugLoc(), TII.get(SPIRV::OpSwitch));
     for (unsigned i = 1; i < I.getNumExplicitOperands(); ++i) {
       if (I.getOperand(i).isReg())
         MIB.addReg(I.getOperand(i).getReg());
       else if (I.getOperand(i).isCImm())
         addNumImm(I.getOperand(i).getCImm()->getValue(), MIB);
       else if (I.getOperand(i).isMBB())
         MIB.addMBB(I.getOperand(i).getMBB());
       else
         llvm_unreachable("Unexpected OpSwitch operand");
     }
     return MIB.constrainAllUses(TII, TRI, RBI);
-  } break;
+  }
   case Intrinsic::spv_cmpxchg:
     return selectAtomicCmpXchg(ResVReg, ResType, I);
-    break;
   case Intrinsic::spv_unreachable:
     BuildMI(BB, I, I.getDebugLoc(), TII.get(SPIRV::OpUnreachable));
     break;
   case Intrinsic::spv_alloca:
     return selectFrameIndex(ResVReg, ResType, I);
-    break;
   default:
     llvm_unreachable("Intrinsic selection not implemented");
   }
   return true;
 }
 
 bool SPIRVInstructionSelector::selectFrameIndex(Register ResVReg,
                                                 const SPIRVType *ResType,

llvm/test/CodeGen/SPIRV/instructions/undef-nested-composite-store.ll

+; RUN: llc -O0 -mtriple=spirv32-unknown-unknown %s -o - | FileCheck %s
+
+; CHECK-DAG: %[[#I32:]] = OpTypeInt 32
+; CHECK-DAG: %[[#I16:]] = OpTypeInt 16
+; CHECK-DAG: %[[#STRUCT:]] = OpTypeStruct %[[#I32]] %[[#I16]]
+; CHECK-DAG: %[[#NESTED_STRUCT:]] = OpTypeStruct %[[#STRUCT]] %[[#I16]]
+; CHECK-DAG: %[[#UNDEF:]] = OpUndef %[[#NESTED_STRUCT]]
+
+; CHECK: %[[#]] = OpFunction %[[#]] None %[[#]]
+; CHECK-NEXT: %[[#PTR:]] = OpFunctionParameter %[[#]]
+; CHECK-NEXT: %[[#]] = OpLabel
+; CHECK-NEXT: OpStore %[[#PTR]] %[[#UNDEF]] Aligned 4
+; CHECK-NEXT: OpReturn
+; CHECK-NEXT: OpFunctionEnd
+
+%struct = type {
+  i32,
+  i16
+}
+
+%nested_struct = type {
+  %struct,
+  i16
+}
+
+define void @foo(ptr %ptr) {
+  store %nested_struct undef, ptr %ptr
+  ret void
+}

llvm/test/CodeGen/SPIRV/instructions/undef-simple-composite-store.ll

+; RUN: llc -O0 -mtriple=spirv32-unknown-unknown %s -o - | FileCheck %s
+
+; CHECK-DAG: %[[#I32:]] = OpTypeInt 32
+; CHECK-DAG: %[[#I16:]] = OpTypeInt 16
+; CHECK-DAG: %[[#STRUCT:]] = OpTypeStruct %[[#I32]] %[[#I16]]
+; CHECK-DAG: %[[#UNDEF:]] = OpUndef %[[#STRUCT]]
+
+; CHECK: %[[#]] = OpFunction %[[#]] None %[[#]]
+; CHECK-NEXT: %[[#PTR:]] = OpFunctionParameter %[[#]]
+; CHECK-NEXT: %[[#]] = OpLabel
+; CHECK-NEXT: OpStore %[[#PTR]] %[[#UNDEF]] Aligned 4
+; CHECK-NEXT: OpReturn
+; CHECK-NEXT: OpFunctionEnd
+
+define void @foo(ptr %ptr) {
+  store { i32, i16 } undef, ptr %ptr
+  ret void
+}