Fold bitcast-to-base into GEP in MergeGepUse, plus refactor and resou…

…rce fixes (microsoft#3801)

* Fold bitcast-to-base into GEP in MergeGepUse, plus refactor

  - Fixes case where bitcast isn't eliminated under -Od.
  - Fix issue with constant GEP path that was unintentionally using GEP
    from failed prior branch.
  - Change to worklist pattern, which simplifies things.
  - One case wasn't handled, which was going to complicate code:
    GEP -> bitcast -> GEP would combine bitcast -> GEP, but not combine
    outer GEP with the new one.
  - Add dxilutil::TryReplaceBaseCastWithGep that simply replaces if it matches.

* Fix cbuffer layouts when resources are in structs

  - keep track of resources in type annotation information
  - fix cbuffer packing locations for things around resources
  - when translating struct for legacy:
    - remove resource fields from struct types
    - properly replace HLSL type
  - remove unused structs and annotations
  - don't produce type annotations for silly things
  - fix other random bugs found along the way

* Fix ValidateVersionNotAllowed test issue with external validator

  It turns out the test wasn't correct in assuming dxil version reported was
  the one it should expect to be supported by the validator, since that
  represents the compiler version.  The validator version constrains the
  dxil version supported by the validator, not the compiler version.
  See comments in the test for details.

* Fix error reporting in ValidationTest::RewriteAssemblyToText

* Fix tests for translated structs and hostlayout name change.

include/dxc/DXIL/DxilTypeSystem.h

@@ -27,7 +27,6 @@ class Function;
 class MDNode;
 class Type;
 class StructType;
-class StringRef;
 }
 
 
@@ -95,7 +94,7 @@ class DxilFieldAnnotation {
   bool m_bCBufferVarUsed; // true if this field represents a top level variable in CB structure, and it is used.
 };
 
-class DxilTemplateArgAnnotation : DxilFieldAnnotation {
+class DxilTemplateArgAnnotation {
 public:
   DxilTemplateArgAnnotation();
 
@@ -126,6 +125,10 @@ class DxilStructAnnotation {
   void SetCBufferSize(unsigned size);
   void MarkEmptyStruct();
   bool IsEmptyStruct();
+  // Since resources don't take real space, IsEmptyBesidesResources
+  // determines if the structure is empty or contains only resources.
+  bool IsEmptyBesidesResources();
+  bool ContainsResources() const;
 
   // For template args, GetNumTemplateArgs() will return 0 if not a template
   unsigned GetNumTemplateArgs() const;
@@ -134,10 +137,15 @@ class DxilStructAnnotation {
   const DxilTemplateArgAnnotation &GetTemplateArgAnnotation(unsigned argIdx) const;
 
 private:
-  const llvm::StructType *m_pStructType;
+  const llvm::StructType *m_pStructType = nullptr;
   std::vector<DxilFieldAnnotation> m_FieldAnnotations;
-  unsigned m_CBufferSize;  // The size of struct if inside constant buffer.
+  unsigned m_CBufferSize = 0;  // The size of struct if inside constant buffer.
   std::vector<DxilTemplateArgAnnotation> m_TemplateAnnotations;
+
+  // m_ResourcesContained property not stored to metadata
+  void SetContainsResources();
+  // HasResources::Only will be set on MarkEmptyStruct() when HasResources::True
+  enum class HasResources { True, False, Only } m_ResourcesContained = HasResources::False;
 };
 
 
@@ -223,10 +231,17 @@ class DxilFunctionAnnotation {
   const llvm::Function *GetFunction() const;
   DxilParameterAnnotation &GetRetTypeAnnotation();
   const DxilParameterAnnotation &GetRetTypeAnnotation() const;
+
+  bool ContainsResourceArgs() { return m_bContainsResourceArgs; }
+
 private:
   const llvm::Function *m_pFunction;
   std::vector<DxilParameterAnnotation> m_parameterAnnotations;
   DxilParameterAnnotation m_retTypeAnnotation;
+
+  // m_bContainsResourceArgs property not stored to metadata
+  void SetContainsResourceArgs() { m_bContainsResourceArgs = true; }
+  bool m_bContainsResourceArgs = false;
 };
 
 /// Use this class to represent structure type annotations in HL and DXIL.
@@ -239,9 +254,11 @@ class DxilTypeSystem {
   DxilTypeSystem(llvm::Module *pModule);
 
   DxilStructAnnotation *AddStructAnnotation(const llvm::StructType *pStructType, unsigned numTemplateArgs = 0);
+  void FinishStructAnnotation(DxilStructAnnotation &SA);
   DxilStructAnnotation *GetStructAnnotation(const llvm::StructType *pStructType);
   const DxilStructAnnotation *GetStructAnnotation(const llvm::StructType *pStructType) const;
   void EraseStructAnnotation(const llvm::StructType *pStructType);
+  void EraseUnusedStructAnnotations();
 
   StructAnnotationMap &GetStructAnnotationMap();
   const StructAnnotationMap &GetStructAnnotationMap() const;
@@ -255,6 +272,7 @@ class DxilTypeSystem {
   const PayloadAnnotationMap &GetPayloadAnnotationMap() const;
 
   DxilFunctionAnnotation *AddFunctionAnnotation(const llvm::Function *pFunction);
+  void FinishFunctionAnnotation(DxilFunctionAnnotation &FA);
   DxilFunctionAnnotation *GetFunctionAnnotation(const llvm::Function *pFunction);
   const DxilFunctionAnnotation *GetFunctionAnnotation(const llvm::Function *pFunction) const;
   void EraseFunctionAnnotation(const llvm::Function *pFunction);
@@ -275,6 +293,9 @@ class DxilTypeSystem {
   bool UseMinPrecision();
   void SetMinPrecision(bool bMinPrecision);
 
+  // Determines whether type is a resource or contains a resource
+  bool IsResourceContained(llvm::Type *Ty);
+
 private:
   llvm::Module *m_pModule;
   StructAnnotationMap m_StructAnnotations;

include/dxc/DXIL/DxilUtil.h

@@ -155,6 +155,12 @@ namespace dxilutil {
 
   bool IsConvergentMarker(llvm::Value *V);
   llvm::Value *GetConvergentSource(llvm::Value *V);
+
+  /// If value is a bitcast to base class pattern, equivalent
+  /// to a getelementptr X, 0, 0, 0...  turn it into the appropriate gep.
+  /// This can enhance SROA and other transforms that want type-safe pointers,
+  /// and enables merging with other getelementptr's.
+  llvm::Value *TryReplaceBaseCastWithGep(llvm::Value *V);
 }
 
 }

lib/DXIL/DxilMetadataHelper.cpp

@@ -842,13 +842,15 @@ void DxilMDHelper::LoadDxilTypeSystemNode(const llvm::MDTuple &MDT,
 
       DxilStructAnnotation *pSA = TypeSystem.AddStructAnnotation(pGVType);
       LoadDxilStructAnnotation(MDT.getOperand(i + 1), *pSA);
+      TypeSystem.FinishStructAnnotation(*pSA);
     }
   } else {
     IFTBOOL((MDT.getNumOperands() & 0x1) == 1, DXC_E_INCORRECT_DXIL_METADATA);
     for (unsigned i = 1; i < MDT.getNumOperands(); i += 2) {
       Function *F = dyn_cast<Function>(ValueMDToValue(MDT.getOperand(i)));
       DxilFunctionAnnotation *pFA = TypeSystem.AddFunctionAnnotation(F);
       LoadDxilFunctionAnnotation(MDT.getOperand(i + 1), *pFA);
+      TypeSystem.FinishFunctionAnnotation(*pFA);
     }
   }
 }

lib/DXIL/DxilTypeSystem.cpp

@@ -9,6 +9,7 @@
 
 #include "dxc/DXIL/DxilTypeSystem.h"
 #include "dxc/DXIL/DxilModule.h"
+#include "dxc/DXIL/DxilUtil.h"
 #include "dxc/Support/Global.h"
 #include "dxc/Support/WinFunctions.h"
 
@@ -149,7 +150,7 @@ bool DxilPayloadFieldAnnotation::HasAnnotations() const {
 // DxilStructAnnotation class methods.
 //
 DxilTemplateArgAnnotation::DxilTemplateArgAnnotation()
-    : DxilFieldAnnotation(), m_Type(nullptr), m_Integral(0)
+    : m_Type(nullptr), m_Integral(0)
 {}
 
 bool DxilTemplateArgAnnotation::IsType() const { return m_Type != nullptr; }
@@ -182,8 +183,26 @@ void DxilStructAnnotation::SetStructType(const llvm::StructType *Ty) {
 
 unsigned DxilStructAnnotation::GetCBufferSize() const { return m_CBufferSize; }
 void DxilStructAnnotation::SetCBufferSize(unsigned size) { m_CBufferSize = size; }
-void DxilStructAnnotation::MarkEmptyStruct() { m_FieldAnnotations.clear(); }
-bool DxilStructAnnotation::IsEmptyStruct() { return m_FieldAnnotations.empty(); }
+void DxilStructAnnotation::MarkEmptyStruct() {
+  if (m_ResourcesContained == HasResources::True)
+    m_ResourcesContained = HasResources::Only;
+  else
+    m_FieldAnnotations.clear();
+}
+bool DxilStructAnnotation::IsEmptyStruct() {
+  return m_FieldAnnotations.empty();
+}
+bool DxilStructAnnotation::IsEmptyBesidesResources() {
+  return m_ResourcesContained == HasResources::Only ||
+         m_FieldAnnotations.empty();
+}
+
+// ContainsResources is for codegen only, not meant for metadata
+void DxilStructAnnotation::SetContainsResources() {
+  if (m_ResourcesContained == HasResources::False)
+    m_ResourcesContained = HasResources::True;
+}
+bool DxilStructAnnotation::ContainsResources() const { return m_ResourcesContained != HasResources::False; }
 
 // For template args, GetNumTemplateArgs() will return 0 if not a template
 unsigned DxilStructAnnotation::GetNumTemplateArgs() const {
@@ -200,7 +219,6 @@ const DxilTemplateArgAnnotation &DxilStructAnnotation::GetTemplateArgAnnotation(
   return m_TemplateAnnotations[argIdx];
 }
 
-
 //------------------------------------------------------------------------------
 //
 // DxilParameterAnnotation class methods.
@@ -297,6 +315,21 @@ DxilStructAnnotation *DxilTypeSystem::AddStructAnnotation(const StructType *pStr
   return pA;
 }
 
+void DxilTypeSystem::FinishStructAnnotation(DxilStructAnnotation &SA) {
+  const llvm::StructType *ST = SA.GetStructType();
+  DXASSERT(SA.GetNumFields() == ST->getNumElements(), "otherwise, mismatched field count.");
+
+  // Update resource containment
+  for (unsigned i = 0; i < SA.GetNumFields() && !SA.ContainsResources(); i++) {
+    if (IsResourceContained(ST->getElementType(i)))
+      SA.SetContainsResources();
+  }
+
+  // Mark if empty
+  if (SA.GetCBufferSize() == 0)
+    SA.MarkEmptyStruct();
+}
+
 DxilStructAnnotation *DxilTypeSystem::GetStructAnnotation(const StructType *pStructType) {
   auto it = m_StructAnnotations.find(pStructType);
   if (it != m_StructAnnotations.end()) {
@@ -323,6 +356,57 @@ void DxilTypeSystem::EraseStructAnnotation(const StructType *pStructType) {
           &I) { return pStructType == I.first; });
 }
 
+// Recurse type, removing any found StructType from the set
+static void RemoveUsedStructsFromSet(Type *Ty, std::unordered_set<const llvm::StructType*> &unused_structs) {
+  if (Ty->isPointerTy())
+    RemoveUsedStructsFromSet(Ty->getPointerElementType(), unused_structs);
+  else if (Ty->isArrayTy())
+    RemoveUsedStructsFromSet(Ty->getArrayElementType(), unused_structs);
+  else if (Ty->isStructTy()) {
+    StructType *ST = cast<StructType>(Ty);
+    // Only recurse first time into this struct
+    if (unused_structs.erase(ST)) {
+      for (auto &ET : ST->elements()) {
+        RemoveUsedStructsFromSet(ET, unused_structs);
+      }
+    }
+  }
+}
+
+void DxilTypeSystem::EraseUnusedStructAnnotations() {
+  // Add all structures with annotations to a set
+  // Iterate globals, resource types, and functions, recursing used structures to
+  // remove matching struct annotations from set
+  std::unordered_set<const llvm::StructType*> unused_structs;
+  for (auto &it : m_StructAnnotations) {
+    unused_structs.insert(it.first);
+  }
+  for (auto &GV : m_pModule->globals()) {
+    RemoveUsedStructsFromSet(GV.getType(), unused_structs);
+  }
+  DxilModule &DM = m_pModule->GetDxilModule();
+  for (auto &&C : DM.GetCBuffers()) {
+    RemoveUsedStructsFromSet(C->GetHLSLType(), unused_structs);
+  }
+  for (auto &&Srv : DM.GetSRVs()) {
+    RemoveUsedStructsFromSet(Srv->GetHLSLType(), unused_structs);
+  }
+  for (auto &&Uav : DM.GetUAVs()) {
+    RemoveUsedStructsFromSet(Uav->GetHLSLType(), unused_structs);
+  }
+  for (auto &F : m_pModule->functions()) {
+    FunctionType *FT = F.getFunctionType();
+    RemoveUsedStructsFromSet(FT->getReturnType(), unused_structs);
+    for (auto &argTy : FT->params()) {
+      RemoveUsedStructsFromSet(argTy, unused_structs);
+    }
+  }
+  // erase remaining structures in set
+  for (auto *ST : unused_structs) {
+    EraseStructAnnotation(ST);
+  }
+}
+
 DxilTypeSystem::StructAnnotationMap &DxilTypeSystem::GetStructAnnotationMap() {
   return m_StructAnnotations;
 }
@@ -383,6 +467,18 @@ DxilFunctionAnnotation *DxilTypeSystem::AddFunctionAnnotation(const Function *pF
   return pA;
 }
 
+void DxilTypeSystem::FinishFunctionAnnotation(DxilFunctionAnnotation &FA) {
+  auto FT = FA.GetFunction()->getFunctionType();
+
+  // Update resource containment
+  if (IsResourceContained(FT->getReturnType()))
+    FA.SetContainsResourceArgs();
+  for (unsigned i = 0; i < FT->getNumParams() && !FA.ContainsResourceArgs(); i++) {
+    if (IsResourceContained(FT->getParamType(i)))
+      FA.SetContainsResourceArgs();
+  }
+}
+
 DxilFunctionAnnotation *DxilTypeSystem::GetFunctionAnnotation(const Function *pFunction) {
   auto it = m_FunctionAnnotations.find(pFunction);
   if (it != m_FunctionAnnotations.end()) {
@@ -659,6 +755,24 @@ void DxilTypeSystem::SetMinPrecision(bool bMinPrecision) {
   m_LowPrecisionMode = mode;
 }
 
+bool DxilTypeSystem::IsResourceContained(llvm::Type *Ty) {
+  // strip pointer/array
+  if (Ty->isPointerTy())
+    Ty = Ty->getPointerElementType();
+  if (Ty->isArrayTy())
+    Ty = Ty->getArrayElementType();
+
+  if (auto ST = dyn_cast<StructType>(Ty)) {
+    if (dxilutil::IsHLSLResourceType(Ty)) {
+      return true;
+    } else if (auto SA = GetStructAnnotation(ST)) {
+      if (SA->ContainsResources())
+        return true;
+    }
+  }
+  return false;
+}
+
 DxilStructTypeIterator::DxilStructTypeIterator(llvm::StructType *sTy, DxilStructAnnotation *sAnnotation,
   unsigned idx)
   : STy(sTy), SAnnotation(sAnnotation), index(idx) {

lib/DXIL/DxilUtil.cpp

@@ -1192,6 +1192,47 @@ Value *GetConvergentSource(Value *V) {
   return cast<CallInst>(V)->getOperand(0);
 }
 
+/// If value is a bitcast to base class pattern, equivalent
+/// to a getelementptr X, 0, 0, 0...  turn it into the appropriate gep.
+/// This can enhance SROA and other transforms that want type-safe pointers,
+/// and enables merging with other getelementptr's.
+Value *TryReplaceBaseCastWithGep(Value *V) {
+  if (BitCastOperator *BCO = dyn_cast<BitCastOperator>(V)) {
+    if (!BCO->getSrcTy()->isPointerTy())
+      return nullptr;
+
+    Type *SrcElTy = BCO->getSrcTy()->getPointerElementType();
+    Type *DstElTy = BCO->getDestTy()->getPointerElementType();
+
+    // Adapted from code in InstCombiner::visitBitCast
+    unsigned NumZeros = 0;
+    while (SrcElTy != DstElTy && isa<CompositeType>(SrcElTy) &&
+           !SrcElTy->isPointerTy() &&
+           SrcElTy->getNumContainedTypes() /* not "{}" */) {
+      SrcElTy = cast<CompositeType>(SrcElTy)->getTypeAtIndex(0U);
+      ++NumZeros;
+    }
+
+    // If we found a path from the src to dest, create the getelementptr now.
+    if (SrcElTy == DstElTy) {
+      IRBuilder<> Builder(BCO->getContext());
+      StringRef Name = "";
+      if (Instruction *I = dyn_cast<Instruction>(BCO)) {
+        Builder.SetInsertPoint(I);
+        Name = I->getName();
+      }
+      SmallVector<Value *, 8> Indices(NumZeros + 1, Builder.getInt32(0));
+      Value *newGEP = Builder.CreateInBoundsGEP(nullptr, BCO->getOperand(0), Indices, Name);
+      V->replaceAllUsesWith(newGEP);
+      if (auto *I = dyn_cast<Instruction>(V))
+        I->eraseFromParent();
+      return newGEP;
+    }
+  }
+
+  return nullptr;
+}
+
 }
 }