pr-feedback: move to GR & alternative lowering

Moved more EmitIntrinsic function to GR, and changed the lowering to use
shuffle and reuse array load logic.

Author: Keenuts

llvm/lib/Target/SPIRV/CMakeLists.txt

@@ -27,7 +27,7 @@ add_llvm_target(SPIRVCodeGen
   SPIRVInstrInfo.cpp
   SPIRVInstructionSelector.cpp
   SPIRVStripConvergentIntrinsics.cpp
-  SPIRVLegalizePointerLoad.cpp
+  SPIRVLegalizePointerCast.cpp
   SPIRVMergeRegionExitTargets.cpp
   SPIRVISelLowering.cpp
   SPIRVLegalizerInfo.cpp

llvm/lib/Target/SPIRV/SPIRV.h

@@ -23,7 +23,7 @@ ModulePass *createSPIRVPrepareFunctionsPass(const SPIRVTargetMachine &TM);
 FunctionPass *createSPIRVStructurizerPass();
 FunctionPass *createSPIRVMergeRegionExitTargetsPass();
 FunctionPass *createSPIRVStripConvergenceIntrinsicsPass();
-FunctionPass *createSPIRVLegalizePointerLoadPass(SPIRVTargetMachine *TM);
+FunctionPass *createSPIRVLegalizePointerCastPass(SPIRVTargetMachine *TM);
 FunctionPass *createSPIRVRegularizerPass();
 FunctionPass *createSPIRVPreLegalizerCombiner();
 FunctionPass *createSPIRVPreLegalizerPass();

llvm/lib/Target/SPIRV/SPIRVEmitIntrinsics.cpp

@@ -139,8 +139,6 @@ class SPIRVEmitIntrinsics
   Type *reconstructType(Value *Op, bool UnknownElemTypeI8,
                         bool IsPostprocessing);
 
-  void buildAssignType(IRBuilder<> &B, Type *ElemTy, Value *Arg);
-
   void replaceMemInstrUses(Instruction *Old, Instruction *New, IRBuilder<> &B);
   void processInstrAfterVisit(Instruction *I, IRBuilder<> &B);
   bool insertAssignPtrTypeIntrs(Instruction *I, IRBuilder<> &B,
@@ -254,18 +252,6 @@ bool expectIgnoredInIRTranslation(const Instruction *I) {
   }
 }
 
-bool allowEmitFakeUse(const Value *Arg) {
-  if (isSpvIntrinsic(Arg))
-    return false;
-  if (dyn_cast<AtomicCmpXchgInst>(Arg) || dyn_cast<InsertValueInst>(Arg) ||
-      dyn_cast<UndefValue>(Arg))
-    return false;
-  if (const auto *LI = dyn_cast<LoadInst>(Arg))
-    if (LI->getType()->isAggregateType())
-      return false;
-  return true;
-}
-
 } // namespace
 
 char SPIRVEmitIntrinsics::ID = 0;
@@ -336,10 +322,7 @@ static void emitAssignName(Instruction *I, IRBuilder<> &B) {
 
 void SPIRVEmitIntrinsics::replaceAllUsesWith(Value *Src, Value *Dest,
                                              bool DeleteOld) {
-  Src->replaceAllUsesWith(Dest);
-  // Update deduced type records
-  GR->updateIfExistDeducedElementType(Src, Dest, DeleteOld);
-  GR->updateIfExistAssignPtrTypeInstr(Src, Dest, DeleteOld);
+  GR->replaceAllUsesWith(Src, Dest, DeleteOld);
   // Update uncomplete type records if any
   if (isTodoType(Src)) {
     if (DeleteOld)
@@ -406,26 +389,6 @@ Type *SPIRVEmitIntrinsics::reconstructType(Value *Op, bool UnknownElemTypeI8,
   return nullptr;
 }
 
-void SPIRVEmitIntrinsics::buildAssignType(IRBuilder<> &B, Type *Ty,
-                                          Value *Arg) {
-  Value *OfType = getNormalizedPoisonValue(Ty);
-  CallInst *AssignCI = nullptr;
-  if (Arg->getType()->isAggregateType() && Ty->isAggregateType() &&
-      allowEmitFakeUse(Arg)) {
-    LLVMContext &Ctx = Arg->getContext();
-    SmallVector<Metadata *, 2> ArgMDs{
-        MDNode::get(Ctx, ValueAsMetadata::getConstant(OfType)),
-        MDString::get(Ctx, Arg->getName())};
-    B.CreateIntrinsic(Intrinsic::spv_value_md, {},
-                      {MetadataAsValue::get(Ctx, MDTuple::get(Ctx, ArgMDs))});
-    AssignCI = B.CreateIntrinsic(Intrinsic::fake_use, {}, {Arg});
-  } else {
-    AssignCI = buildIntrWithMD(Intrinsic::spv_assign_type, {Arg->getType()},
-                               OfType, Arg, {}, B);
-  }
-  GR->addAssignPtrTypeInstr(Arg, AssignCI);
-}
-
 CallInst *SPIRVEmitIntrinsics::buildSpvPtrcast(Function *F, Value *Op,
                                                Type *ElemTy) {
   IRBuilder<> B(Op->getContext());
@@ -1453,7 +1416,7 @@ void SPIRVEmitIntrinsics::insertAssignPtrTypeTargetExt(
 
   CallInst *AssignCI = GR->findAssignPtrTypeInstr(V);
   if (!AssignCI) {
-    buildAssignType(B, AssignedType, V);
+    GR->buildAssignType(B, AssignedType, V);
     return;
   }
 
@@ -1907,8 +1870,8 @@ void SPIRVEmitIntrinsics::insertAssignTypeIntrs(Instruction *I,
         setInsertPointAfterDef(B, I);
         switch (ResIt->second) {
         case WellKnownTypes::Event:
-          buildAssignType(B, TargetExtType::get(I->getContext(), "spirv.Event"),
-                          I);
+          GR->buildAssignType(
+              B, TargetExtType::get(I->getContext(), "spirv.Event"), I);
           break;
         }
       }
@@ -1953,7 +1916,7 @@ void SPIRVEmitIntrinsics::insertAssignTypeIntrs(Instruction *I,
       }
     }
     TypeToAssign = restoreMutatedType(GR, I, TypeToAssign);
-    buildAssignType(B, TypeToAssign, I);
+    GR->buildAssignType(B, TypeToAssign, I);
   }
   for (const auto &Op : I->operands()) {
     if (isa<ConstantPointerNull>(Op) || isa<UndefValue>(Op) ||

llvm/lib/Target/SPIRV/SPIRVGlobalRegistry.cpp

@@ -32,6 +32,20 @@
 
 using namespace llvm;
 
+namespace {
+
+bool allowEmitFakeUse(const Value *Arg) {
+  if (isSpvIntrinsic(Arg))
+    return false;
+  if (dyn_cast<AtomicCmpXchgInst>(Arg) || dyn_cast<InsertValueInst>(Arg) ||
+      dyn_cast<UndefValue>(Arg))
+    return false;
+  if (const auto *LI = dyn_cast<LoadInst>(Arg))
+    if (LI->getType()->isAggregateType())
+      return false;
+  return true;
+}
+
 inline unsigned typeToAddressSpace(const Type *Ty) {
   if (auto PType = dyn_cast<TypedPointerType>(Ty))
     return PType->getAddressSpace();
@@ -43,6 +57,8 @@ inline unsigned typeToAddressSpace(const Type *Ty) {
   report_fatal_error("Unable to convert LLVM type to SPIRVType", true);
 }
 
+} // anonymous namespace
+
 SPIRVGlobalRegistry::SPIRVGlobalRegistry(unsigned PointerSize)
     : PointerSize(PointerSize), Bound(0) {}
 
@@ -1743,6 +1759,33 @@ LLT SPIRVGlobalRegistry::getRegType(SPIRVType *SpvType) const {
   return LLT::scalar(64);
 }
 
+void SPIRVGlobalRegistry::replaceAllUsesWith(Value *Old, Value *New,
+                                             bool DeleteOld) {
+  Old->replaceAllUsesWith(New);
+  updateIfExistDeducedElementType(Old, New, DeleteOld);
+  updateIfExistAssignPtrTypeInstr(Old, New, DeleteOld);
+}
+
+void SPIRVGlobalRegistry::buildAssignType(IRBuilder<> &B, Type *Ty,
+                                          Value *Arg) {
+  Value *OfType = getNormalizedPoisonValue(Ty);
+  CallInst *AssignCI = nullptr;
+  if (Arg->getType()->isAggregateType() && Ty->isAggregateType() &&
+      allowEmitFakeUse(Arg)) {
+    LLVMContext &Ctx = Arg->getContext();
+    SmallVector<Metadata *, 2> ArgMDs{
+        MDNode::get(Ctx, ValueAsMetadata::getConstant(OfType)),
+        MDString::get(Ctx, Arg->getName())};
+    B.CreateIntrinsic(Intrinsic::spv_value_md, {},
+                      {MetadataAsValue::get(Ctx, MDTuple::get(Ctx, ArgMDs))});
+    AssignCI = B.CreateIntrinsic(Intrinsic::fake_use, {}, {Arg});
+  } else {
+    AssignCI = buildIntrWithMD(Intrinsic::spv_assign_type, {Arg->getType()},
+                               OfType, Arg, {}, B);
+  }
+  addAssignPtrTypeInstr(Arg, AssignCI);
+}
+
 void SPIRVGlobalRegistry::buildAssignPtr(IRBuilder<> &B, Type *ElemTy,
                                          Value *Arg) {
   Value *OfType = PoisonValue::get(ElemTy);

llvm/lib/Target/SPIRV/SPIRVGlobalRegistry.h

@@ -621,6 +621,11 @@ class SPIRVGlobalRegistry {
   const TargetRegisterClass *getRegClass(SPIRVType *SpvType) const;
   LLT getRegType(SPIRVType *SpvType) const;
 
+  // Replace all uses of a |Old| with |New| updates the global registry type
+  // mappings.
+  void replaceAllUsesWith(Value *Old, Value *New, bool DeleteOld = true);
+
+  void buildAssignType(IRBuilder<> &B, Type *Ty, Value *Arg);
   void buildAssignPtr(IRBuilder<> &B, Type *ElemTy, Value *Arg);
   void updateAssignType(CallInst *AssignCI, Value *Arg, Value *OfType);
 };

llvm/lib/Target/SPIRV/SPIRVLegalizePointerLoad.cpp renamed to llvm/lib/Target/SPIRV/SPIRVLegalizePointerCast.cpp

@@ -1,4 +1,4 @@
-//===-- SPIRVLegalizePointerLoad.cpp ----------------------*- C++ -*-===//
+//===-- SPIRVLegalizePointerCast.cpp ----------------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
@@ -13,8 +13,8 @@
 // by logical SPIR-V.
 //
 // This pass relies on the assign_ptr_type intrinsic to deduce the type of the
-// pointee. All occurrences of `ptrcast` must be replaced because the lead to
-// invalid SPIR-V. Unhandled cases result in an error.
+// pointed values, must replace all occurences of `ptrcast`. This is why
+// unhandled cases are reported as unreachable: we MUST cover all cases.
 //
 // 1. Loading the first element of an array
 //
@@ -57,18 +57,10 @@
 using namespace llvm;
 
 namespace llvm {
-void initializeSPIRVLegalizePointerLoadPass(PassRegistry &);
+void initializeSPIRVLegalizePointerCastPass(PassRegistry &);
 }
 
-class SPIRVLegalizePointerLoad : public FunctionPass {
-
-  // Replace all uses of a |Old| with |New| updates the global registry type
-  // mappings.
-  void replaceAllUsesWith(Value *Old, Value *New) {
-    Old->replaceAllUsesWith(New);
-    GR->updateIfExistDeducedElementType(Old, New, /* deleteOld = */ true);
-    GR->updateIfExistAssignPtrTypeInstr(Old, New, /* deleteOld = */ true);
-  }
+class SPIRVLegalizePointerCast : public FunctionPass {
 
   // Builds the `spv_assign_type` assigning |Ty| to |Value| at the current
   // builder position.
@@ -79,89 +71,54 @@ class SPIRVLegalizePointerLoad : public FunctionPass {
     GR->addAssignPtrTypeInstr(Arg, AssignCI);
   }
 
-  // Loads a single scalar of type |To| from the vector pointed by |Source| of
-  // the type |From|. Returns the loaded value.
-  Value *loadScalarFromVector(IRBuilder<> &B, Value *Source,
-                              FixedVectorType *From) {
-
-    LoadInst *NewLoad = B.CreateLoad(From, Source);
-    buildAssignType(B, From, NewLoad);
-
-    SmallVector<Value *, 2> Args = {NewLoad, B.getInt64(0)};
-    SmallVector<Type *, 3> Types = {From->getElementType(), Args[0]->getType(),
-                                    Args[1]->getType()};
-    Value *Extracted =
-        B.CreateIntrinsic(Intrinsic::spv_extractelt, {Types}, {Args});
-    buildAssignType(B, Extracted->getType(), Extracted);
-    return Extracted;
-  }
-
-  // Loads parts of the vector of type |From| from the pointer |Source| and
-  // create a new vector of type |To|. |To| must be a vector type, and element
-  // types of |To| and |From| must match. Returns the loaded value.
-  Value *loadVectorFromVector(IRBuilder<> &B, FixedVectorType *From,
-                              FixedVectorType *To, Value *Source) {
+  // Loads parts of the vector of type |SourceType| from the pointer |Source|
+  // and create a new vector of type |TargetType|. |TargetType| must be a vector
+  // type, and element types of |TargetType| and |SourceType| must match.
+  // Returns the loaded value.
+  Value *loadVectorFromVector(IRBuilder<> &B, FixedVectorType *SourceType,
+                              FixedVectorType *TargetType, Value *Source) {
     // We expect the codegen to avoid doing implicit bitcast from a load.
-    assert(To->getElementType() == From->getElementType());
-    assert(To->getNumElements() < From->getNumElements());
-
-    LoadInst *NewLoad = B.CreateLoad(From, Source);
-    buildAssignType(B, From, NewLoad);
+    assert(TargetType->getElementType() == SourceType->getElementType());
+    assert(TargetType->getNumElements() < SourceType->getNumElements());
 
-    auto ConstInt = ConstantInt::get(IntegerType::get(B.getContext(), 32), 0);
-    ElementCount VecElemCount = ElementCount::getFixed(To->getNumElements());
-    Value *Output = ConstantVector::getSplat(VecElemCount, ConstInt);
-    for (unsigned I = 0; I < To->getNumElements(); ++I) {
-      Value *Extracted = nullptr;
-      {
-        SmallVector<Value *, 2> Args = {NewLoad, B.getInt64(I)};
-        SmallVector<Type *, 3> Types = {To->getElementType(),
-                                        Args[0]->getType(), Args[1]->getType()};
-        Extracted =
-            B.CreateIntrinsic(Intrinsic::spv_extractelt, {Types}, {Args});
-        buildAssignType(B, Extracted->getType(), Extracted);
-      }
-      assert(Extracted != nullptr);
+    LoadInst *NewLoad = B.CreateLoad(SourceType, Source);
+    buildAssignType(B, SourceType, NewLoad);
 
-      {
-        SmallVector<Value *, 3> Args = {Output, Extracted, B.getInt64(I)};
-        SmallVector<Type *, 4> Types = {Args[0]->getType(), Args[0]->getType(),
-                                        Args[1]->getType(), Args[2]->getType()};
-        Output = B.CreateIntrinsic(Intrinsic::spv_insertelt, {Types}, {Args});
-        buildAssignType(B, Output->getType(), Output);
-      }
-    }
+    SmallVector<int> Mask(/* Size= */ TargetType->getNumElements(),
+                          /* Value= */ 0);
+    Value *Output = B.CreateShuffleVector(NewLoad, NewLoad, Mask);
+    buildAssignType(B, TargetType, Output);
     return Output;
   }
 
-  // Loads the first value in an array pointed by |Source| of type |From|. Load
-  // flags will be copied from |BadLoad|, which should be the illegal load being
-  // legalized. Returns the loaded value.
-  Value *loadFirstValueFromArray(IRBuilder<> &B, ArrayType *From, Value *Source,
-                                 LoadInst *BadLoad) {
+  // Loads the first value in an aggregate pointed by |Source| of containing
+  // elements of type |ElementType|. Load flags will be copied from |BadLoad|,
+  // which should be the load being legalized. Returns the loaded value.
+  Value *loadFirstValueFromAggregate(IRBuilder<> &B, Type *ElementType,
+                                     Value *Source, LoadInst *BadLoad) {
     SmallVector<Type *, 2> Types = {BadLoad->getPointerOperandType(),
                                     BadLoad->getPointerOperandType()};
-    SmallVector<Value *, 3> Args{/* isInBounds= */ B.getInt1(true), Source,
-                                 B.getInt64(0), B.getInt64(0)};
+    SmallVector<Value *, 3> Args{/* isInBounds= */ B.getInt1(false), Source,
+                                 B.getInt32(0), B.getInt32(0)};
     auto *GEP = B.CreateIntrinsic(Intrinsic::spv_gep, {Types}, {Args});
-    GR->buildAssignPtr(B, From->getElementType(), GEP);
+    GR->buildAssignPtr(B, ElementType, GEP);
 
     const auto *TLI = TM->getSubtargetImpl()->getTargetLowering();
     MachineMemOperand::Flags Flags = TLI->getLoadMemOperandFlags(
         *BadLoad, BadLoad->getFunction()->getDataLayout());
     Instruction *LI = B.CreateIntrinsic(
         Intrinsic::spv_load, {BadLoad->getOperand(0)->getType()},
         {GEP, B.getInt16(Flags), B.getInt8(BadLoad->getAlign().value())});
-    buildAssignType(B, From->getElementType(), LI);
+    buildAssignType(B, ElementType, LI);
     return LI;
   }
 
-  // Transforms an illegal partial load into a sequence we can lower to logical
-  // SPIR-V.
+  // Replaces the load instruction to get rid of the ptrcast used as source
+  // operand.
   void transformLoad(IRBuilder<> &B, LoadInst *LI, Value *CastedOperand,
                      Value *OriginalOperand) {
     Type *FromTy = GR->findDeducedElementType(OriginalOperand);
-    Type *ToTy /*-fruity*/ = GR->findDeducedElementType(CastedOperand);
+    Type *ToTy = GR->findDeducedElementType(CastedOperand);
     Value *Output = nullptr;
 
     auto *SAT = dyn_cast<ArrayType>(FromTy);
@@ -174,12 +131,14 @@ class SPIRVLegalizePointerLoad : public FunctionPass {
     // Loading 1st element.
     // - float a = array[0];
     if (SAT && SAT->getElementType() == ToTy)
-      Output = loadFirstValueFromArray(B, SAT, OriginalOperand, LI);
+      Output = loadFirstValueFromAggregate(B, SAT->getElementType(),
+                                           OriginalOperand, LI);
     // Destination is the element type of Source, and source is a vector ->
     // Vector to scalar.
     // - float a = vector.x;
     else if (!DVT && SVT && SVT->getElementType() == ToTy) {
-      Output = loadScalarFromVector(B, OriginalOperand, SVT);
+      Output = loadFirstValueFromAggregate(B, SVT->getElementType(),
+                                           OriginalOperand, LI);
     }
     // Destination is a smaller vector than source.
     // - float3 v3 = vector4;
@@ -188,7 +147,7 @@ class SPIRVLegalizePointerLoad : public FunctionPass {
     else
       llvm_unreachable("Unimplemented implicit down-cast from load.");
 
-    replaceAllUsesWith(LI, Output);
+    GR->replaceAllUsesWith(LI, Output, /* DeleteOld= */ true);
     DeadInstructions.push_back(LI);
   }
 
@@ -220,8 +179,8 @@ class SPIRVLegalizePointerLoad : public FunctionPass {
   }
 
 public:
-  SPIRVLegalizePointerLoad(SPIRVTargetMachine *TM) : FunctionPass(ID), TM(TM) {
-    initializeSPIRVLegalizePointerLoadPass(*PassRegistry::getPassRegistry());
+  SPIRVLegalizePointerCast(SPIRVTargetMachine *TM) : FunctionPass(ID), TM(TM) {
+    initializeSPIRVLegalizePointerCastPass(*PassRegistry::getPassRegistry());
   };
 
   virtual bool runOnFunction(Function &F) override {
@@ -256,10 +215,10 @@ class SPIRVLegalizePointerLoad : public FunctionPass {
   static char ID;
 };
 
-char SPIRVLegalizePointerLoad::ID = 0;
-INITIALIZE_PASS(SPIRVLegalizePointerLoad, "spirv-legalize-bitcast",
+char SPIRVLegalizePointerCast::ID = 0;
+INITIALIZE_PASS(SPIRVLegalizePointerCast, "spirv-legalize-bitcast",
                 "SPIRV legalize bitcast pass", false, false)
 
-FunctionPass *llvm::createSPIRVLegalizePointerLoadPass(SPIRVTargetMachine *TM) {
-  return new SPIRVLegalizePointerLoad(TM);
+FunctionPass *llvm::createSPIRVLegalizePointerCastPass(SPIRVTargetMachine *TM) {
+  return new SPIRVLegalizePointerCast(TM);
 }