diff --git a/include/dxc/DXIL/DxilTypeSystem.h b/include/dxc/DXIL/DxilTypeSystem.h index d5640fe0a..c55e598be 100644 --- a/include/dxc/DXIL/DxilTypeSystem.h +++ b/include/dxc/DXIL/DxilTypeSystem.h @@ -27,7 +27,6 @@ class Function; class MDNode; class Type; class StructType; -class StringRef; } @@ -95,7 +94,7 @@ private: 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 @@ public: 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 @@ public: const DxilTemplateArgAnnotation &GetTemplateArgAnnotation(unsigned argIdx) const; private: - const llvm::StructType *m_pStructType; + const llvm::StructType *m_pStructType = nullptr; std::vector 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 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 @@ public: const llvm::Function *GetFunction() const; DxilParameterAnnotation &GetRetTypeAnnotation(); const DxilParameterAnnotation &GetRetTypeAnnotation() const; + + bool ContainsResourceArgs() { return m_bContainsResourceArgs; } + private: const llvm::Function *m_pFunction; std::vector 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 @@ public: 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 @@ public: 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 @@ public: 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; diff --git a/include/dxc/DXIL/DxilUtil.h b/include/dxc/DXIL/DxilUtil.h index 73a362e49..642db0446 100644 --- a/include/dxc/DXIL/DxilUtil.h +++ b/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); } } diff --git a/lib/DXIL/DxilMetadataHelper.cpp b/lib/DXIL/DxilMetadataHelper.cpp index 618613fd6..704c48d4a 100644 --- a/lib/DXIL/DxilMetadataHelper.cpp +++ b/lib/DXIL/DxilMetadataHelper.cpp @@ -842,6 +842,7 @@ 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); @@ -849,6 +850,7 @@ void DxilMDHelper::LoadDxilTypeSystemNode(const llvm::MDTuple &MDT, Function *F = dyn_cast(ValueMDToValue(MDT.getOperand(i))); DxilFunctionAnnotation *pFA = TypeSystem.AddFunctionAnnotation(F); LoadDxilFunctionAnnotation(MDT.getOperand(i + 1), *pFA); + TypeSystem.FinishFunctionAnnotation(*pFA); } } } diff --git a/lib/DXIL/DxilTypeSystem.cpp b/lib/DXIL/DxilTypeSystem.cpp index 21088a3f6..b47f3c35c 100644 --- a/lib/DXIL/DxilTypeSystem.cpp +++ b/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 &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(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 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(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) { diff --git a/lib/DXIL/DxilUtil.cpp b/lib/DXIL/DxilUtil.cpp index d8f78cf74..6a8fe957d 100644 --- a/lib/DXIL/DxilUtil.cpp +++ b/lib/DXIL/DxilUtil.cpp @@ -1192,6 +1192,47 @@ Value *GetConvergentSource(Value *V) { return cast(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(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(SrcElTy) && + !SrcElTy->isPointerTy() && + SrcElTy->getNumContainedTypes() /* not "{}" */) { + SrcElTy = cast(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(BCO)) { + Builder.SetInsertPoint(I); + Name = I->getName(); + } + SmallVector Indices(NumZeros + 1, Builder.getInt32(0)); + Value *newGEP = Builder.CreateInBoundsGEP(nullptr, BCO->getOperand(0), Indices, Name); + V->replaceAllUsesWith(newGEP); + if (auto *I = dyn_cast(V)) + I->eraseFromParent(); + return newGEP; + } + } + + return nullptr; +} + } } diff --git a/lib/HLSL/DxilCondenseResources.cpp b/lib/HLSL/DxilCondenseResources.cpp index d12a9642a..a890195cd 100644 --- a/lib/HLSL/DxilCondenseResources.cpp +++ b/lib/HLSL/DxilCondenseResources.cpp @@ -581,6 +581,9 @@ public: dxilutil::RemoveUnusedFunctions(M, DM.GetEntryFunction(), DM.GetPatchConstantFunction(), m_bIsLib); + // Erase type annotations for structures no longer used + DM.GetTypeSystem().EraseUnusedStructAnnotations(); + return bChanged; } @@ -1620,7 +1623,8 @@ bool DxilLowerCreateHandleForLib::RemovePhiOnResource() { namespace { StructType *UpdateStructTypeForLegacyLayout(StructType *ST, - DxilTypeSystem &TypeSys, Module &M); + DxilTypeSystem &TypeSys, Module &M, + bool includeTopLevelResource=false); Type *UpdateFieldTypeForLegacyLayout(Type *Ty, DxilFieldAnnotation &annotation, @@ -1633,8 +1637,10 @@ Type *UpdateFieldTypeForLegacyLayout(Type *Ty, UpdateFieldTypeForLegacyLayout(EltTy, annotation, TypeSys, M); if (EltTy == UpdatedTy) return Ty; - else + else if (UpdatedTy) return ArrayType::get(UpdatedTy, Ty->getArrayNumElements()); + else + return nullptr; } else if (hlsl::HLMatrixType::isa(Ty)) { DXASSERT(annotation.HasMatrixAnnotation(), "must a matrix"); HLMatrixType MatTy = HLMatrixType::cast(Ty); @@ -1690,12 +1696,17 @@ Type *UpdateFieldTypeForLegacyLayout(Type *Ty, StructType *UpdateStructTypeForLegacyLayout(StructType *ST, DxilTypeSystem &TypeSys, - Module &M) { + Module &M, + bool includeTopLevelResource) { bool bUpdated = false; unsigned fieldsCount = ST->getNumElements(); - std::vector fieldTypes(fieldsCount); + std::vector fieldTypes; + fieldTypes.reserve(fieldsCount); DxilStructAnnotation *SA = TypeSys.GetStructAnnotation(ST); + if (!includeTopLevelResource && dxilutil::IsHLSLResourceType(ST)) + return nullptr; + // After reflection is stripped from library, this will be null if no update is required. if (!SA) { return ST; @@ -1705,11 +1716,20 @@ StructType *UpdateStructTypeForLegacyLayout(StructType *ST, return ST; } + // Resource fields must be deleted, since they don't actually + // show up in the structure layout. + // fieldMap maps from new field index to old field index for porting annotations + std::vector fieldMap; + fieldMap.reserve(fieldsCount); + for (unsigned i = 0; i < fieldsCount; i++) { Type *EltTy = ST->getElementType(i); Type *UpdatedTy = UpdateFieldTypeForLegacyLayout( EltTy, SA->GetFieldAnnotation(i), TypeSys, M); - fieldTypes[i] = UpdatedTy; + if (UpdatedTy != nullptr) { + fieldMap.push_back(i); + fieldTypes.push_back(UpdatedTy); + } if (EltTy != UpdatedTy) bUpdated = true; } @@ -1723,12 +1743,24 @@ StructType *UpdateStructTypeForLegacyLayout(StructType *ST, StructType *NewST = StructType::create(ST->getContext(), fieldTypes, legacyName); - DxilStructAnnotation *NewSA = TypeSys.AddStructAnnotation(NewST); - // Clone annotation. - *NewSA = *SA; - // Make sure we set the struct type back to the new one, since the - // clone would have clobbered it with the old one. - NewSA->SetStructType(NewST); + + // Only add annotation if struct is not empty. + if (NewST->getNumElements() > 0) { + DxilStructAnnotation *NewSA = TypeSys.AddStructAnnotation(NewST); + + // Clone annotation. + NewSA->SetCBufferSize(SA->GetCBufferSize()); + NewSA->SetNumTemplateArgs(SA->GetNumTemplateArgs()); + for (unsigned i = 0; i < SA->GetNumTemplateArgs(); i++) { + NewSA->GetTemplateArgAnnotation(i) = SA->GetTemplateArgAnnotation(i); + } + // Remap with deleted resource fields + for (unsigned i = 0; i < NewSA->GetNumFields(); i++) { + NewSA->GetFieldAnnotation(i) = SA->GetFieldAnnotation(fieldMap[i]); + } + TypeSys.FinishStructAnnotation(*NewSA); + } + return NewST; } } @@ -1749,13 +1781,15 @@ bool UpdateStructTypeForLegacyLayout(DxilResourceBase &Res, } Type *UpdatedST = - UpdateStructTypeForLegacyLayout(ST, TypeSys, M); + UpdateStructTypeForLegacyLayout(ST, TypeSys, M, + Res.GetKind() == DXIL::ResourceKind::StructuredBuffer); if (ST != UpdatedST) { // Support Array of ConstantBuffer/StructuredBuffer. UpdatedST = dxilutil::WrapInArrayTypes(UpdatedST, arrayDims); GlobalVariable *NewGV = cast( M.getOrInsertGlobal(Symbol->getName().str() + "_legacy", UpdatedST)); Res.SetGlobalSymbol(NewGV); + Res.SetHLSLType(NewGV->getType()); OP *hlslOP = DM.GetOP(); if (DM.GetShaderModel()->IsLib()) { @@ -1862,6 +1896,8 @@ void DxilLowerCreateHandleForLib::UpdateResourceSymbols() { GV->removeDeadConstantUsers(); DXASSERT(GV->user_empty(), "else resource not lowered"); res->SetGlobalSymbol(UndefValue::get(GV->getType())); + if (GV->user_empty()) + GV->eraseFromParent(); } }; diff --git a/lib/HLSL/DxilContainerReflection.cpp b/lib/HLSL/DxilContainerReflection.cpp index f7f6cf87c..11465508d 100644 --- a/lib/HLSL/DxilContainerReflection.cpp +++ b/lib/HLSL/DxilContainerReflection.cpp @@ -1097,7 +1097,7 @@ HRESULT CShaderReflectionType::Initialize( // There is no annotation for empty structs unsigned int fieldCount = 0; - if (structAnnotation) + if (structAnnotation && !structAnnotation->IsEmptyBesidesResources()) fieldCount = type->getStructNumElements(); // The DXBC reflection info computes `Columns` for a @@ -1128,6 +1128,13 @@ HRESULT CShaderReflectionType::Initialize( m_MemberTypes.push_back(fieldReflectionType); m_MemberNames.push_back(fieldAnnotation.GetFieldName().c_str()); + // Skip structures fields with no real contents, otherwise we expand + // the size of this struct by 1 when we treat a zero column size as 1. + if (isa(fieldType) && + fieldReflectionType->m_Desc.Columns == 0) { + continue; + } + // Effectively, we want to add one to `Columns` for every scalar nested recursively // inside this `struct` type (ignoring objects, which we filtered above). We should // be able to compute this as the product of the `Columns`, `Rows` and `Elements` diff --git a/lib/HLSL/HLModule.cpp b/lib/HLSL/HLModule.cpp index 683e497e7..a55cc2de8 100644 --- a/lib/HLSL/HLModule.cpp +++ b/lib/HLSL/HLModule.cpp @@ -980,8 +980,13 @@ void HLModule::GetParameterRowsAndCols(Type *Ty, unsigned &rows, unsigned &cols, rows *= arraySize; } -static Value *MergeGEP(GEPOperator *SrcGEP, GetElementPtrInst *GEP) { - IRBuilder<> Builder(GEP); +static Value *MergeGEP(GEPOperator *SrcGEP, GEPOperator *GEP) { + IRBuilder<> Builder(GEP->getContext()); + StringRef Name = ""; + if (Instruction *I = dyn_cast(GEP)) { + Builder.SetInsertPoint(I); + Name = GEP->getName(); + } SmallVector Indices; // Find out whether the last index in the source GEP is a sequential idx. @@ -1028,51 +1033,52 @@ static Value *MergeGEP(GEPOperator *SrcGEP, GetElementPtrInst *GEP) { Indices.append(SrcGEP->op_begin() + 1, SrcGEP->op_end()); Indices.append(GEP->idx_begin() + 1, GEP->idx_end()); } - if (!Indices.empty()) - return Builder.CreateInBoundsGEP(SrcGEP->getSourceElementType(), - SrcGEP->getOperand(0), Indices, - GEP->getName()); - else - llvm_unreachable("must merge"); + + DXASSERT(!Indices.empty(), "must merge"); + Value *newGEP = Builder.CreateInBoundsGEP(nullptr, SrcGEP->getOperand(0), Indices, Name); + GEP->replaceAllUsesWith(newGEP); + if (Instruction *I = dyn_cast(GEP)) + I->eraseFromParent(); + return newGEP; } void HLModule::MergeGepUse(Value *V) { - for (auto U = V->user_begin(); U != V->user_end();) { - auto Use = U++; - - if (GetElementPtrInst *GEP = dyn_cast(*Use)) { - if (GEPOperator *prevGEP = dyn_cast(V)) { - // merge the 2 GEPs - Value *newGEP = MergeGEP(prevGEP, GEP); - // Don't need to replace when GEP is updated in place - if (newGEP != GEP) { - GEP->replaceAllUsesWith(newGEP); - GEP->eraseFromParent(); - } - MergeGepUse(newGEP); + SmallVector worklist; + auto addUsersToWorklist = [&worklist](Value *V) { + if (!V->user_empty()) { + // Add users in reverse to the worklist, so they are processed in order + // This makes it equivalent to recursive traversal + size_t start = worklist.size(); + worklist.append(V->user_begin(), V->user_end()); + size_t end = worklist.size(); + std::reverse(worklist.data() + start, worklist.data() + end); + } + }; + addUsersToWorklist(V); + while (worklist.size()) { + V = worklist.pop_back_val(); + if (BitCastOperator *BCO = dyn_cast(V)) { + if (Value *NewV = dxilutil::TryReplaceBaseCastWithGep(V)) { + worklist.push_back(NewV); } else { - MergeGepUse(*Use); + // merge any GEP users of the untranslated bitcast + addUsersToWorklist(V); } - } else if (dyn_cast(*Use)) { - if (GEPOperator *prevGEP = dyn_cast(V)) { - // merge the 2 GEPs - Value *newGEP = MergeGEP(prevGEP, GEP); - // Don't need to replace when GEP is updated in place - if (newGEP != GEP) { - GEP->replaceAllUsesWith(newGEP); - GEP->eraseFromParent(); + } else if (GEPOperator *GEP = dyn_cast(V)) { + if (GEPOperator *prevGEP = dyn_cast(GEP->getPointerOperand())) { + // merge the 2 GEPs, returns nullptr if couldn't merge + if (Value *newGEP = MergeGEP(prevGEP, GEP)) { + worklist.push_back(newGEP); + // delete prevGEP if no more users + if (prevGEP->user_empty() && isa(prevGEP)) + cast(prevGEP)->eraseFromParent(); } - MergeGepUse(newGEP); } else { - MergeGepUse(*Use); + // nothing to merge yet, add GEP users + addUsersToWorklist(V); } } } - if (V->user_empty()) { - // Only remove GEP here, root ptr will be removed by DCE. - if (GetElementPtrInst *I = dyn_cast(V)) - I->eraseFromParent(); - } } template diff --git a/tools/clang/include/clang/AST/HlslTypes.h b/tools/clang/include/clang/AST/HlslTypes.h index a6ce119e0..c3acf3dec 100644 --- a/tools/clang/include/clang/AST/HlslTypes.h +++ b/tools/clang/include/clang/AST/HlslTypes.h @@ -394,6 +394,7 @@ bool IsHLSLTriangleStreamType(clang::QualType type); bool IsHLSLStreamOutputType(clang::QualType type); bool IsHLSLResourceType(clang::QualType type); bool IsHLSLBufferViewType(clang::QualType type); +bool IsHLSLStructuredBufferType(clang::QualType type); bool IsHLSLNumericOrAggregateOfNumericType(clang::QualType type); bool IsHLSLNumericUserDefinedType(clang::QualType type); bool IsHLSLAggregateType(clang::QualType type); diff --git a/tools/clang/lib/AST/HlslTypes.cpp b/tools/clang/lib/AST/HlslTypes.cpp index bab7e839f..646424290 100644 --- a/tools/clang/lib/AST/HlslTypes.cpp +++ b/tools/clang/lib/AST/HlslTypes.cpp @@ -562,6 +562,18 @@ bool IsHLSLBufferViewType(clang::QualType type) { return false; } +bool IsHLSLStructuredBufferType(clang::QualType type) { + if (const RecordType *RT = type->getAs()) { + StringRef name = RT->getDecl()->getName(); + if (name == "StructuredBuffer" || name == "RWStructuredBuffer") + return true; + + if (name == "AppendStructuredBuffer" || name == "ConsumeStructuredBuffer") + return true; + } + return false; +} + bool IsHLSLSubobjectType(clang::QualType type) { DXIL::SubobjectKind kind; DXIL::HitGroupType hgType; diff --git a/tools/clang/lib/CodeGen/CGHLSLMS.cpp b/tools/clang/lib/CodeGen/CGHLSLMS.cpp index b6f60beb2..34a30a493 100644 --- a/tools/clang/lib/CodeGen/CGHLSLMS.cpp +++ b/tools/clang/lib/CodeGen/CGHLSLMS.cpp @@ -204,7 +204,7 @@ private: // Type annotation related. unsigned ConstructStructAnnotation(DxilStructAnnotation *annotation, - DxilPayloadAnnotation* payloadAnnotation, + DxilPayloadAnnotation *payloadAnnotation, const RecordDecl *RD, DxilTypeSystem &dxilTypeSys); unsigned AddTypeAnnotation(QualType Ty, DxilTypeSystem &dxilTypeSys, @@ -870,6 +870,10 @@ static void ConstructFieldInterpolation(DxilFieldAnnotation &fieldAnnotation, static unsigned AlignBaseOffset(unsigned baseOffset, unsigned size, QualType Ty, bool bDefaultRowMajor) { + // Do not align if resource, since resource isn't really here. + if (IsHLSLResourceType(Ty)) + return baseOffset; + bool needNewAlign = Ty->isArrayType(); if (IsHLSLMatType(Ty)) { @@ -924,7 +928,7 @@ unsigned CGMSHLSLRuntime::ConstructStructAnnotation(DxilStructAnnotation *annota const RecordDecl *RD, DxilTypeSystem &dxilTypeSys) { unsigned fieldIdx = 0; - unsigned offset = 0; + unsigned CBufferOffset = 0; bool bDefaultRowMajor = m_pHLModule->GetHLOptions().bDefaultRowMajor; if (const CXXRecordDecl *CXXRD = dyn_cast(RD)) { @@ -957,46 +961,47 @@ unsigned CGMSHLSLRuntime::ConstructStructAnnotation(DxilStructAnnotation *annota QualType parentTy = QualType(BaseDecl->getTypeForDecl(), 0); - // Align offset. - offset = AlignBaseOffset(parentTy, offset, bDefaultRowMajor, CGM, - dataLayout); - - unsigned CBufferOffset = offset; - - unsigned arrayEltSize = 0; // Process field to make sure the size of field is ready. - unsigned size = - AddTypeAnnotation(parentTy, dxilTypeSys, arrayEltSize); + unsigned arrayEltSize = 0; + unsigned size = AddTypeAnnotation(parentTy, dxilTypeSys, arrayEltSize); - // Update offset. - offset += size; + // Align offset. + if (size) + CBufferOffset = AlignBaseOffset(parentTy, CBufferOffset, bDefaultRowMajor, CGM, + dataLayout); - if (size > 0) { + llvm::StructType *baseType = + cast(CGM.getTypes().ConvertType(parentTy)); + DxilStructAnnotation *baseAnnotation = + dxilTypeSys.GetStructAnnotation(baseType); + + if (size || (baseAnnotation && !baseAnnotation->IsEmptyStruct())) { DxilFieldAnnotation &fieldAnnotation = annotation->GetFieldAnnotation(fieldIdx++); fieldAnnotation.SetCBufferOffset(CBufferOffset); fieldAnnotation.SetFieldName(BaseDecl->getNameAsString()); } + + // Update offset. + CBufferOffset += size; } } } + unsigned CBufferSize = CBufferOffset; + for (auto fieldDecl : RD->fields()) { std::string fieldSemName = ""; QualType fieldTy = fieldDecl->getType(); DXASSERT(!fieldDecl->isBitField(), "We should have already ensured we have no bitfields."); - // Align offset. - offset = AlignBaseOffset(fieldTy, offset, bDefaultRowMajor, CGM, dataLayout); - - unsigned CBufferOffset = offset; - DxilFieldAnnotation &fieldAnnotation = annotation->GetFieldAnnotation(fieldIdx++); ConstructFieldAttributedAnnotation(fieldAnnotation, fieldTy, bDefaultRowMajor); // Try to get info from fieldDecl. + const hlsl::ConstantPacking *packOffset = nullptr; for (const hlsl::UnusualAnnotation *it : fieldDecl->getUnusualAnnotations()) { switch (it->getKind()) { @@ -1005,9 +1010,9 @@ unsigned CGMSHLSLRuntime::ConstructStructAnnotation(DxilStructAnnotation *annota fieldSemName = sd->SemanticName; } break; case hlsl::UnusualAnnotation::UA_ConstantPacking: { - const hlsl::ConstantPacking *cp = cast(it); - CBufferOffset = cp->Subcomponent << 2; - CBufferOffset += cp->ComponentOffset; + packOffset = cast(it); + CBufferOffset = packOffset->Subcomponent << 2; + CBufferOffset += packOffset->ComponentOffset; // Change to byte. CBufferOffset <<= 2; } break; @@ -1041,13 +1046,25 @@ unsigned CGMSHLSLRuntime::ConstructStructAnnotation(DxilStructAnnotation *annota } } - unsigned arrayEltSize = 0; // Process field to make sure the size of field is ready. + unsigned arrayEltSize = 0; unsigned size = AddTypeAnnotation(fieldDecl->getType(), dxilTypeSys, arrayEltSize); - // Update offset. - offset += size; - + // Align offset. + if (size) { + unsigned offset = AlignBaseOffset(fieldTy, CBufferOffset, bDefaultRowMajor, CGM, dataLayout); + if (packOffset && CBufferOffset != offset) { + // custom offset has an alignment problem, or this code does + DiagnosticsEngine &Diags = CGM.getDiags(); + unsigned DiagID = Diags.getCustomDiagID( + DiagnosticsEngine::Error, + "custom offset mis-aligned."); + Diags.Report(packOffset->Loc, DiagID); + return 0; + } + CBufferOffset = offset; + } + ConstructFieldInterpolation(fieldAnnotation, fieldDecl); if (fieldDecl->hasAttr()) fieldAnnotation.SetPrecise(); @@ -1060,13 +1077,15 @@ unsigned CGMSHLSLRuntime::ConstructStructAnnotation(DxilStructAnnotation *annota if (m_PreciseOutputSet.count(StringRef(fieldSemName).lower())) fieldAnnotation.SetPrecise(); } + + // Update offset. + CBufferSize = std::max(CBufferSize, CBufferOffset + size); + CBufferOffset = CBufferSize; } - annotation->SetCBufferSize(offset); - if (offset == 0) { - annotation->MarkEmptyStruct(); - } - return offset; + annotation->SetCBufferSize(CBufferSize); + dxilTypeSys.FinishStructAnnotation(*annotation); + return CBufferSize; } static bool IsElementInputOutputType(QualType Ty) { @@ -1147,6 +1166,9 @@ static bool ValidatePayloadDecl(const RecordDecl *Decl, unsigned CGMSHLSLRuntime::AddTypeAnnotation(QualType Ty, DxilTypeSystem &dxilTypeSys, unsigned &arrayEltSize) { + if (Ty.isNull()) + return 0; + QualType paramTy = Ty.getCanonicalType(); if (const ReferenceType *RefType = dyn_cast(paramTy)) paramTy = RefType->getPointeeType(); @@ -1173,7 +1195,12 @@ unsigned CGMSHLSLRuntime::AddTypeAnnotation(QualType Ty, else if (IsHLSLOutputPatchType(Ty)) return AddTypeAnnotation(GetHLSLOutputPatchElementType(Ty), dxilTypeSys, arrayEltSize); - else if (const RecordType *RT = paramTy->getAsStructureType()) { + else if (!IsHLSLStructuredBufferType(Ty) && IsHLSLResourceType(Ty)) { + // Save result type info. + AddTypeAnnotation(GetHLSLResourceResultType(Ty), dxilTypeSys, arrayEltSize); + // Resources don't count towards cbuffer size. + return 0; + } else if (const RecordType *RT = paramTy->getAsStructureType()) { RecordDecl *RD = RT->getDecl(); llvm::StructType *ST = CGM.getTypes().ConvertRecordDeclType(RD); // Skip if already created. @@ -1186,7 +1213,9 @@ unsigned CGMSHLSLRuntime::AddTypeAnnotation(QualType Ty, DxilPayloadAnnotation *payloadAnnotation = nullptr; if (ValidatePayloadDecl(RT->getDecl(), *m_pHLModule->GetShaderModel(), CGM.getDiags(), CGM.getCodeGenOpts())) payloadAnnotation = dxilTypeSys.AddPayloadAnnotation(ST); - return ConstructStructAnnotation(annotation, payloadAnnotation, RD, dxilTypeSys); + unsigned size = ConstructStructAnnotation(annotation, payloadAnnotation, RD, dxilTypeSys); + // Resources don't count towards cbuffer size. + return IsHLSLResourceType(Ty) ? 0 : size; } else if (const RecordType *RT = dyn_cast(paramTy)) { // For this pointer. RecordDecl *RD = RT->getDecl(); @@ -1202,11 +1231,6 @@ unsigned CGMSHLSLRuntime::AddTypeAnnotation(QualType Ty, if (ValidatePayloadDecl(RT->getDecl(), *m_pHLModule->GetShaderModel(), CGM.getDiags(), CGM.getCodeGenOpts())) payloadAnnotation = dxilTypeSys.AddPayloadAnnotation(ST); return ConstructStructAnnotation(annotation, payloadAnnotation, RD, dxilTypeSys); - } else if (IsHLSLResourceType(Ty)) { - // Save result type info. - AddTypeAnnotation(GetHLSLResourceResultType(Ty), dxilTypeSys, arrayEltSize); - // Resource don't count for cbuffer size. - return 0; } else if (IsStringType(Ty)) { // string won't be included in cbuffer return 0; @@ -2298,20 +2322,24 @@ void CGMSHLSLRuntime::AddHLSLFunctionInfo(Function *F, const FunctionDecl *FD) { return; // Type annotation for parameters and return type. - DxilTypeSystem &dxilTypeSys = m_pHLModule->GetTypeSystem(); - unsigned arrayEltSize = 0; - AddTypeAnnotation(FD->getReturnType(), dxilTypeSys, arrayEltSize); + { + DxilTypeSystem &dxilTypeSys = m_pHLModule->GetTypeSystem(); + unsigned arrayEltSize = 0; + AddTypeAnnotation(FD->getReturnType(), dxilTypeSys, arrayEltSize); - // Type annotation for this pointer. - if (const CXXMethodDecl *MFD = dyn_cast(FD)) { - const CXXRecordDecl *RD = MFD->getParent(); - QualType Ty = CGM.getContext().getTypeDeclType(RD); - AddTypeAnnotation(Ty, dxilTypeSys, arrayEltSize); - } + // Type annotation for this pointer. + if (const CXXMethodDecl *MFD = dyn_cast(FD)) { + const CXXRecordDecl *RD = MFD->getParent(); + QualType Ty = CGM.getContext().getTypeDeclType(RD); + AddTypeAnnotation(Ty, dxilTypeSys, arrayEltSize); + } - for (const ValueDecl *param : FD->params()) { - QualType Ty = param->getType(); - AddTypeAnnotation(Ty, dxilTypeSys, arrayEltSize); + for (const ValueDecl *param : FD->params()) { + QualType Ty = param->getType(); + AddTypeAnnotation(Ty, dxilTypeSys, arrayEltSize); + } + + dxilTypeSys.FinishFunctionAnnotation(*FuncAnnotation); } // clear isExportedEntry if not exporting entry diff --git a/tools/clang/lib/CodeGen/CGHLSLMSFinishCodeGen.cpp b/tools/clang/lib/CodeGen/CGHLSLMSFinishCodeGen.cpp index 13ecf6d03..77d7ef2ee 100644 --- a/tools/clang/lib/CodeGen/CGHLSLMSFinishCodeGen.cpp +++ b/tools/clang/lib/CodeGen/CGHLSLMSFinishCodeGen.cpp @@ -1984,6 +1984,10 @@ unsigned AlignCBufferOffset(unsigned offset, unsigned size, llvm::Type *Ty, bool bRowMajor, bool bMinPrecMode, bool &bCurRowIsMinPrec) { DXASSERT(!(offset & 1), "otherwise we have an invalid offset."); + // resources, empty structure, or structures with only resources have + // zero size, and need no alignment. + if (size == 0) + return offset; bool bNeedNewRow = Ty->isArrayTy(); // In min-precision mode, a new row is needed when // going into or out of min-precision component type. diff --git a/tools/clang/test/HLSLFileCheck/hlsl/resource_binding/bindings1.hlsl b/tools/clang/test/HLSLFileCheck/hlsl/resource_binding/bindings1.hlsl index 6c7a2708e..5062dfc9d 100644 --- a/tools/clang/test/HLSLFileCheck/hlsl/resource_binding/bindings1.hlsl +++ b/tools/clang/test/HLSLFileCheck/hlsl/resource_binding/bindings1.hlsl @@ -93,8 +93,6 @@ // CHECK: ; RWTex4 UAV f32 2d U2 u17 6 // CHECK: ; RWTex1 UAV f32 2d U3 u0 4 -// CHECK: %struct.Resources = type { %"class.Texture2D", %"class.Texture2D >", %"class.Texture2D", %"class.Texture2D >", %"class.RWTexture2D >", %"class.RWTexture2D >", %"class.RWTexture2D >", %"class.RWTexture2D >", %struct.SamplerComparisonState, %struct.SamplerState, %struct.SamplerComparisonState, %struct.SamplerState, <4 x float> } - // CHK60: %[[RWTex2:.*]] = call %dx.types.Handle @dx.op.createHandle(i32 57, i8 1, i32 0, i32 7, i1 false) // CHK60: %[[MyTB:.*]] = call %dx.types.Handle @dx.op.createHandle(i32 57, i8 0, i32 4, i32 11, i1 false) diff --git a/tools/clang/test/HLSLFileCheck/hlsl/resource_binding/resource_in_cbuffer.hlsl b/tools/clang/test/HLSLFileCheck/hlsl/resource_binding/resource_in_cbuffer.hlsl index c48f13a31..856833d95 100644 --- a/tools/clang/test/HLSLFileCheck/hlsl/resource_binding/resource_in_cbuffer.hlsl +++ b/tools/clang/test/HLSLFileCheck/hlsl/resource_binding/resource_in_cbuffer.hlsl @@ -1,24 +1,61 @@ // RUN: %dxc -E main -T ps_6_0 %s | FileCheck %s -// CHECK:tx0.s sampler NA NA S0 s0 1 -// CHECK:tx1.s sampler NA NA S1 s1 1 -// CHECK:tx0.t texture f32 2d T0 t0 1 -// CHECK:tx1.t texture f32 2d T1 t1 1 + +// Make sure structures with only resources have resource fields removed, +// are considered empty, take no space in CBuffer, and do not force alignment. +// CHECK: cbuffer $Globals +// CHECK: struct hostlayout.$Globals +// CHECK: float h; ; Offset: 0 +// CHECK: struct hostlayout.struct.LegacyTex +// CHECK: /* empty struct */ +// CHECK: } tx1; ; Offset: 4 +// CHECK: float i; ; Offset: 4 +// CHECK: } $Globals; ; Offset: 0 Size: 8 +// CHECK: cbuffer CB0 +// CHECK: struct hostlayout.CB0 +// CHECK: float f; ; Offset: 0 +// CHECK: struct hostlayout.struct.LegacyTex +// CHECK: /* empty struct */ +// CHECK: } tx0; ; Offset: 4 +// CHECK: float g; ; Offset: 4 +// CHECK: } CB0; ; Offset: 0 Size: 8 + +// CHECK: $Globals cbuffer NA NA CB0 cb0 1 +// CHECK: CB0 cbuffer NA NA CB1 cb1 1 + +// TODO: Preserve delcaration order when allocating resources from structs +// CHECK-DAG: tx0.s sampler NA NA S{{.}} s{{.}} 1 +// CHECK-DAG: tx1.s sampler NA NA S{{.}} s{{.}} 1 +// CHECK-DAG: b0 texture f32 buf T{{.}} t{{.}} 1 +// CHECK-DAG: b1 texture f32 buf T{{.}} t{{.}} 1 +// CHECK-DAG: tx0.t texture f32 2d T{{.}} t{{.}} 1 +// CHECK-DAG: tx1.t texture f32 2d T{{.}} t{{.}} 1 + +// bound the check-dags +// CHECK: define void @main() { struct LegacyTex { - Texture2D t; - SamplerState s; + Texture2D t; + SamplerState s; }; -LegacyTex tx0; -LegacyTex tx1; +cbuffer CB0 { + float f; // CB0[0].x + LegacyTex tx0; // t0, s0 + float g; // CB0[0].y +} +Buffer b0; // t1 +float h; // $Globals[0].x +LegacyTex tx1; // t2, s1 +float i; // $Globals[0].y +Buffer b1; // t3 float4 tex2D(LegacyTex tx, float2 uv) { - return tx.t.Sample(tx.s,uv); + return tx.t.Sample(tx.s,uv); } float4 main(float2 uv:UV) : SV_Target { - return tex2D(tx0,uv) + tex2D(tx1,uv); -} \ No newline at end of file + return h * b0[(uint)uv.y] * tex2D(tx1,uv) + b1[(uint)uv.x] * tex2D(tx0,uv) * f + g + i; +} diff --git a/tools/clang/test/HLSLFileCheck/hlsl/resource_binding/resource_in_cbuffer2.hlsl b/tools/clang/test/HLSLFileCheck/hlsl/resource_binding/resource_in_cbuffer2.hlsl new file mode 100644 index 000000000..12ce6d425 --- /dev/null +++ b/tools/clang/test/HLSLFileCheck/hlsl/resource_binding/resource_in_cbuffer2.hlsl @@ -0,0 +1,61 @@ +// RUN: %dxc -E main -T ps_6_0 %s | FileCheck %s + +// Make sure structures with resources have resource fields removed. +// CHECK: cbuffer $Globals +// CHECK: struct hostlayout.$Globals +// CHECK: float h; ; Offset: 0 +// CHECK: struct hostlayout.struct.LegacyTex +// CHECK: float f; ; Offset: 16 +// CHECK: } tx1; ; Offset: 16 +// CHECK: float i; ; Offset: 20 +// CHECK: } $Globals; ; Offset: 0 Size: 24 +// CHECK: cbuffer CB0 +// CHECK: struct hostlayout.CB0 +// CHECK: float f; ; Offset: 0 +// CHECK: struct hostlayout.struct.LegacyTex +// CHECK: float f; ; Offset: 16 +// CHECK: } tx0; ; Offset: 16 +// CHECK: float g; ; Offset: 20 +// CHECK: } CB0; ; Offset: 0 Size: 24 + +// CHECK: $Globals cbuffer NA NA CB0 cb0 1 +// CHECK: CB0 cbuffer NA NA CB1 cb1 1 + +// TODO: Preserve delcaration order when allocating resources from structs +// CHECK-DAG: tx0.s sampler NA NA S{{.}} s{{.}} 1 +// CHECK-DAG: tx1.s sampler NA NA S{{.}} s{{.}} 1 +// CHECK-DAG: b0 texture f32 buf T{{.}} t{{.}} 1 +// CHECK-DAG: b1 texture f32 buf T{{.}} t{{.}} 1 +// CHECK-DAG: tx0.t texture f32 2d T{{.}} t{{.}} 1 +// CHECK-DAG: tx1.t texture f32 2d T{{.}} t{{.}} 1 + +// bound the check-dags +// CHECK: define void @main() { + +struct LegacyTex +{ + Texture2D t; + float f; + SamplerState s; +}; + +cbuffer CB0 { + float f; // CB0[0].x + LegacyTex tx0; // t0, s0, CB0[1].x + float g; // CB0[2].x +} +Buffer b0; // t1 +float h; // $Globals[0].x +LegacyTex tx1; // t2, s1, $Globals[1].x +float i; // $Globals[2].x +Buffer b1; // t3 + +float4 tex2D(LegacyTex tx, float2 uv) +{ + return tx.t.Sample(tx.s,uv) * tx.f; +} + +float4 main(float2 uv:UV) : SV_Target +{ + return h * b0[(uint)uv.y] * tex2D(tx1,uv) + b1[(uint)uv.x] * tex2D(tx0,uv) * f + g * i; +} diff --git a/tools/clang/test/HLSLFileCheck/hlsl/types/matrix/default-matrix-in-template.hlsl b/tools/clang/test/HLSLFileCheck/hlsl/types/matrix/default-matrix-in-template.hlsl index fcff1bd70..3a50a4102 100644 --- a/tools/clang/test/HLSLFileCheck/hlsl/types/matrix/default-matrix-in-template.hlsl +++ b/tools/clang/test/HLSLFileCheck/hlsl/types/matrix/default-matrix-in-template.hlsl @@ -1,5 +1,6 @@ -// RUN: %dxc -E main -T cs_6_0 %s | FileCheck %s +// RUN: %dxc -E main -T cs_6_0 -fcgl %s | FileCheck %s +// Check unlowered type // CHECK: %"class.StructuredBuffer >" = type { %class.matrix.float.4.4 } StructuredBuffer buf1; diff --git a/tools/clang/test/HLSLFileCheck/shader_targets/library/lib_hs_shaders_only.hlsl b/tools/clang/test/HLSLFileCheck/shader_targets/library/lib_hs_shaders_only.hlsl index f9a4e1a4b..bff6a45b9 100644 --- a/tools/clang/test/HLSLFileCheck/shader_targets/library/lib_hs_shaders_only.hlsl +++ b/tools/clang/test/HLSLFileCheck/shader_targets/library/lib_hs_shaders_only.hlsl @@ -1,6 +1,5 @@ // RUN: %dxc -auto-binding-space 13 -T lib_6_3 -export-shaders-only %s | FileCheck %s -// CHECK: class.Buffer // CHECK-NOT: unused // CHECK-NOT: @"\01?HSPerPatchFunc1@@YA?AUHSPerPatchData@@V?$InputPatch@UPSSceneIn@@$0BA@@@@Z" // CHECK: define void @"\01?HSPerPatchFunc2 diff --git a/tools/clang/test/HLSLFileCheck/shader_targets/library/lib_shaders_only.hlsl b/tools/clang/test/HLSLFileCheck/shader_targets/library/lib_shaders_only.hlsl index 4ad09dc5e..2bcb7a6be 100644 --- a/tools/clang/test/HLSLFileCheck/shader_targets/library/lib_shaders_only.hlsl +++ b/tools/clang/test/HLSLFileCheck/shader_targets/library/lib_shaders_only.hlsl @@ -1,7 +1,6 @@ // RUN: %dxc -auto-binding-space 13 -T lib_6_3 -export-shaders-only %s | FileCheck %s // CHECK: %struct.Payload -// CHECK: class.Buffer // CHECK-NOT: unused // CHECK: define void @"\01?AnyHit // CHECK: define void @"\01?Callable diff --git a/tools/clang/test/HLSLFileCheck/shader_targets/raytracing/raytracing_derived_payload.hlsl b/tools/clang/test/HLSLFileCheck/shader_targets/raytracing/raytracing_derived_payload.hlsl new file mode 100644 index 000000000..74a1b65f2 --- /dev/null +++ b/tools/clang/test/HLSLFileCheck/shader_targets/raytracing/raytracing_derived_payload.hlsl @@ -0,0 +1,47 @@ +// RUN: %dxc -T lib_6_3 -Od %s | FileCheck %s + +// Verifies packoffset of struct with multiple levels of inheritance +// CHECK: } cb_pld; ; Offset: 32 +// CHECK: float f; ; Offset: 4 +// CHECK: } CB; ; Offset: 0 Size: 72 + +// Make sure bitcast to base for `this` as part of the method call gets folded into GEP on -Od +// CHECK: define void [[raygen1:@"\\01\?raygen1@[^\"]+"]]() #0 { +// CHECK-NOT: bitcast +// CHECK: ret void + +struct SuperBasePayload { + float g; + bool DoInner() { return g < 0; } +}; + +struct BasePayload : SuperBasePayload { + float f; + bool DoSomething() { return f < 0 && DoInner(); } +}; + +struct MyPayload : BasePayload { + int i; + float4 color; + uint2 pos; +}; + +RaytracingAccelerationStructure RTAS : register(t5); +RWByteAddressBuffer BAB : register(u0) ; + +cbuffer CB : register(b0) { + MyPayload cb_pld : packoffset(c2.x); + float f : packoffset(c0.y); +} + +[shader("raygeneration")] +void raygen1() +{ + MyPayload p = cb_pld; + p.pos = DispatchRaysIndex(); + float3 origin = {0, 0, 0}; + float3 dir = normalize(float3(p.pos / (float)DispatchRaysDimensions(), 1)); + RayDesc ray = { origin, 0.125, dir, 128.0}; + TraceRay(RTAS, RAY_FLAG_NONE, 0, 0, 1, 0, ray, p); + BAB.Store(p.pos.x + p.pos.y * DispatchRaysDimensions().x, p.DoSomething()); +} diff --git a/tools/clang/unittests/HLSL/DxilContainerTest.cpp b/tools/clang/unittests/HLSL/DxilContainerTest.cpp index ecb82e79e..e8b1ed632 100644 --- a/tools/clang/unittests/HLSL/DxilContainerTest.cpp +++ b/tools/clang/unittests/HLSL/DxilContainerTest.cpp @@ -204,7 +204,13 @@ public: std::string baseName(baseDesc.Name); if (baseName.compare(0, 4, "half", 4) == 0) baseName = baseName.replace(0, 4, "float", 5); - VERIFY_ARE_EQUAL_STR(testDesc.Name, baseName.c_str()); + + // For anonymous structures, fxc uses "scope::", + // dxc uses "anon", and may have additional ".#" for name disambiguation. + // Just skip name check if struct is unnamed according to fxc. + if (baseName.find("") == std::string::npos) { + VERIFY_ARE_EQUAL_STR(testDesc.Name, baseName.c_str()); + } for (UINT i = 0; i < baseDesc.Members; ++i) { ID3D12ShaderReflectionType* testMemberType = pTest->GetMemberTypeByIndex(i); @@ -1800,6 +1806,7 @@ TEST_F(DxilContainerTest, ReflectionMatchesDXBC_CheckIn) { ReflectionTest(hlsl_test::GetPathToHlslDataFile(L"..\\HLSLFileCheck\\d3dreflect\\tbuffer.hlsl").c_str(), false, D3DCOMPILE_ENABLE_BACKWARDS_COMPATIBILITY); ReflectionTest(hlsl_test::GetPathToHlslDataFile(L"..\\HLSLFileCheck\\d3dreflect\\texture2dms.hlsl").c_str(), false); + ReflectionTest(hlsl_test::GetPathToHlslDataFile(L"..\\HLSLFileCheck\\hlsl\\objects\\StructuredBuffer\\layout.hlsl").c_str(), false); } TEST_F(DxilContainerTest, ReflectionMatchesDXBC_Full) { diff --git a/tools/clang/unittests/HLSL/ValidationTest.cpp b/tools/clang/unittests/HLSL/ValidationTest.cpp index 1019c6d0e..3c4ab5385 100644 --- a/tools/clang/unittests/HLSL/ValidationTest.cpp +++ b/tools/clang/unittests/HLSL/ValidationTest.cpp @@ -489,9 +489,22 @@ public: if (bRegex) { llvm::Regex RE(pLookFor); std::string reErrors; + if (!RE.isValid(reErrors)) { + WEX::Logging::Log::Comment(WEX::Common::String().Format( + L"Regex errors:\r\n%.*S\r\nWhile compiling expression '%S'", + (unsigned)reErrors.size(), reErrors.data(), + pLookFor)); + } VERIFY_IS_TRUE(RE.isValid(reErrors)); std::string replaced = RE.sub(pReplacement, disassembly, &reErrors); if (!bOptional) { + if (!reErrors.empty()) { + WEX::Logging::Log::Comment(WEX::Common::String().Format( + L"Regex errors:\r\n%.*S\r\nWhile searching for '%S' in text:\r\n%.*S", + (unsigned)reErrors.size(), reErrors.data(), + pLookFor, + (unsigned)disassembly.size(), disassembly.data())); + } VERIFY_ARE_NOT_EQUAL(disassembly, replaced); VERIFY_IS_TRUE(reErrors.empty()); } @@ -510,6 +523,12 @@ public: pos += replaceLen; } if (!bOptional) { + if (!found) { + WEX::Logging::Log::Comment(WEX::Common::String().Format( + L"String not found: '%S' in text:\r\n%.*S", + pLookFor, + (unsigned)disassembly.size(), disassembly.data())); + } VERIFY_IS_TRUE(found); } } @@ -719,17 +738,17 @@ TEST_F(ValidationTest, BarrierFail) { {"dx.op.barrier(i32 80, i32 8)", "dx.op.barrier(i32 80, i32 9)", "dx.op.barrier(i32 80, i32 11)", - "%\"class.RWStructuredBuffer >\" = type { %class.matrix.float.2.2 }\n", + "%\"hostlayout.class.RWStructuredBuffer >\" = type { [2 x <2 x float>] }\n", "call i32 @dx.op.flattenedThreadIdInGroup.i32(i32 96)", }, {"dx.op.barrier(i32 80, i32 15)", "dx.op.barrier(i32 80, i32 0)", "dx.op.barrier(i32 80, i32 %rem)", - "%\"class.RWStructuredBuffer >\" = type { %class.matrix.float.2.2 }\n" - "@dx.typevar.8 = external addrspace(1) constant %\"class.RWStructuredBuffer >\"\n" + "%\"hostlayout.class.RWStructuredBuffer >\" = type { [2 x <2 x float>] }\n" + "@dx.typevar.8 = external addrspace(1) constant %\"hostlayout.class.RWStructuredBuffer >\"\n" "@\"internalGV\" = internal global [64 x <4 x float>] undef\n", "call i32 @dx.op.flattenedThreadIdInGroup.i32(i32 96)\n" - "%load = load %\"class.RWStructuredBuffer >\", %\"class.RWStructuredBuffer >\" addrspace(1)* @dx.typevar.8", + "%load = load %\"hostlayout.class.RWStructuredBuffer >\", %\"hostlayout.class.RWStructuredBuffer >\" addrspace(1)* @dx.typevar.8", }, {"Internal declaration 'internalGV' is unused", "External declaration 'dx.typevar.8' is unused", @@ -3842,22 +3861,30 @@ TEST_F(ValidationTest, ValidatePrintfNotAllowed) { TEST_F(ValidationTest, ValidateVersionNotAllowed) { if (m_ver.SkipDxilVersion(1, 6)) return; - std::string maxValMinor = std::to_string(m_ver.m_ValMinor); - std::string higherValMinor = std::to_string(m_ver.m_ValMinor + 1); - std::string maxDxilMinor = std::to_string(m_ver.m_DxilMinor); - std::string higherDxilMinor = std::to_string(m_ver.m_DxilMinor + 1); + // When validator version is < dxil verrsion, compiler has a newer version + // than validator. We are checking the validator, so only use the validator + // version. + // This will also assume that the versions are tied together. This has always + // been the case, but it's not assumed that it has to be the case. If the + // versions diverged, it would be impossible to tell what DXIL version a + // validator supports just from the version returned in the IDxcVersion + // interface, without separate knowledge of the supported dxil version based + // on the validator version. If these versions must diverge in the future, we + // could rev the IDxcVersion interface to accomodate. + std::string maxMinor = std::to_string(m_ver.m_ValMinor); + std::string higherMinor = std::to_string(m_ver.m_ValMinor + 1); RewriteAssemblyCheckMsg(L"..\\CodeGenHLSL\\basic.hlsl", "ps_6_0", - ("= !{i32 1, i32 " + maxValMinor + "}").c_str(), - ("= !{i32 1, i32 " + higherValMinor + "}").c_str(), - ("error: Validator version in metadata (1." + higherValMinor + ") is not supported; maximum: (1." + maxValMinor + ")").c_str()); + ("= !{i32 1, i32 " + maxMinor + "}").c_str(), + ("= !{i32 1, i32 " + higherMinor + "}").c_str(), + ("error: Validator version in metadata (1." + higherMinor + ") is not supported; maximum: (1." + maxMinor + ")").c_str()); RewriteAssemblyCheckMsg(L"..\\CodeGenHLSL\\basic.hlsl", "ps_6_0", "= !{i32 1, i32 0}", "= !{i32 1, i32 1}", "error: Shader model requires Dxil Version 1.0"); RewriteAssemblyCheckMsg(L"..\\CodeGenHLSL\\basic.hlsl", "ps_6_0", "= !{i32 1, i32 0}", - ("= !{i32 1, i32 " + higherDxilMinor + "}").c_str(), - ("error: Dxil version in metadata (1." + higherDxilMinor + ") is not supported; maximum: (1." + maxDxilMinor + ")").c_str()); + ("= !{i32 1, i32 " + higherMinor + "}").c_str(), + ("error: Dxil version in metadata (1." + higherMinor + ") is not supported; maximum: (1." + maxMinor + ")").c_str()); } TEST_F(ValidationTest, CreateHandleNotAllowedSM66) {