Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Report error for unsupported types of SV semantics #3043

Merged
merged 24 commits into from
Feb 26, 2021
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
5ae34bf
Report error on unsupported types for CS input semantics
Jul 16, 2020
772847e
Undo changes in SemaHLSL
Jul 16, 2020
8e245ac
Offline feedback from Tex
Jul 17, 2020
8575cf8
Update test
Jul 17, 2020
8771fb4
Fix travis build error
Jul 17, 2020
79bb9bb
Use DXASSERT
Jul 17, 2020
f6f9ebd
Handle matrix
Jul 17, 2020
1f628e1
Fix indentation
Jul 17, 2020
35266e9
Revert previous change
Dec 24, 2020
4006a27
Generate error for disallowed types for CS input semantics
Dec 24, 2020
9a80b70
Merge branch 'master' of https://github.com/microsoft/DirectXShaderCo…
Dec 25, 2020
953457e
Revert "Generate error for disallowed types for CS input semantics"
Feb 9, 2021
1238ade
Merge branch 'master' of https://github.com/microsoft/DirectXShaderCo…
Feb 9, 2021
34d0e50
Extend type checking for all semantics
Feb 9, 2021
fda69bb
Fix travis build error
Feb 9, 2021
581fe30
Use dxilutil::IsIntegerOrFloatingPointType
Feb 11, 2021
61f2575
Merge branch 'master' of https://github.com/microsoft/DirectXShaderCo…
vcsharma Feb 17, 2021
5bac07c
Merge branch 'master' of https://github.com/microsoft/DirectXShaderCo…
vcsharma Feb 22, 2021
2ec8f4f
Merge branch 'master' of https://github.com/microsoft/DirectXShaderCo…
vcsharma Feb 23, 2021
51bfb18
Accomodate change in signature for dxilutil::EmitErrorOnFunction
vcsharma Feb 23, 2021
1c4767f
Review comments
vcsharma Feb 23, 2021
29c2101
Relax the SV type checking criteria based on internal testing
vcsharma Feb 24, 2021
533a4dd
Merge branch 'master' of https://github.com/microsoft/DirectXShaderCo…
vcsharma Feb 25, 2021
53d077f
Review comments
vcsharma Feb 25, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 20 additions & 3 deletions include/dxc/DXIL/DxilSemantic.h
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,19 @@ class Semantic {
public:
using Kind = DXIL::SemanticKind;

enum class CompClass {
Any, // Arbitrary values, for instance
Uint, // 32-bit int or uint allowed
Float, // float, min16float, or float16_t allowed
Bool, // bool allowed
};

enum class SizeClass {
Other, // Arbitrary, or needs special casing (clip/cull/tessfactors)
Scalar, Vec2, Vec3, Vec4, // Exact size only
MaxVec2, MaxVec3, MaxVec4 // Up to this size allowed
};

static const int kUndefinedRow = -1;
static const int kUndefinedCol = -1;

Expand All @@ -41,13 +54,17 @@ class Semantic {
const char *GetName() const;
bool IsArbitrary() const;
bool IsInvalid() const;
CompClass GetCompClass() const { return m_CompClass; }
SizeClass GetSizeClass() const { return m_SizeClass; }

private:
Kind m_Kind; // Semantic kind.
const char *m_pszName; // Canonical name (for system semantics).
Kind m_Kind; // Semantic kind.
const char *m_pszName; // Canonical name (for system semantics).
CompClass m_CompClass; // Allowed component type class
SizeClass m_SizeClass; // Allowed size

Semantic() = delete;
Semantic(Kind Kind, const char *pszName);
Semantic(Kind Kind, const char *pszName, CompClass compClass, SizeClass sizeClass);

// Table of all semantic properties.
static const unsigned kNumSemanticRecords = (unsigned)Kind::Invalid + 1;
Expand Down
72 changes: 38 additions & 34 deletions lib/DXIL/DxilSemantic.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,13 @@ namespace hlsl {
// Semantic class methods.
//
Semantic::Semantic(Kind Kind,
const char *pszName)
const char *pszName,
CompClass compClass,
SizeClass sizeClass)
: m_Kind(Kind)
, m_pszName(pszName)
, m_CompClass(compClass)
, m_SizeClass(sizeClass)
{
}

Expand Down Expand Up @@ -115,39 +119,39 @@ bool Semantic::IsInvalid() const {

typedef Semantic SP;
const Semantic Semantic::ms_SemanticTable[kNumSemanticRecords] = {
// Kind Name
SP(Kind::Arbitrary, nullptr),
SP(Kind::VertexID, "SV_VertexID"),
SP(Kind::InstanceID, "SV_InstanceID"),
SP(Kind::Position, "SV_Position"),
SP(Kind::RenderTargetArrayIndex,"SV_RenderTargetArrayIndex"),
SP(Kind::ViewPortArrayIndex, "SV_ViewportArrayIndex"),
SP(Kind::ClipDistance, "SV_ClipDistance"),
SP(Kind::CullDistance, "SV_CullDistance"),
SP(Kind::OutputControlPointID, "SV_OutputControlPointID"),
SP(Kind::DomainLocation, "SV_DomainLocation"),
SP(Kind::PrimitiveID, "SV_PrimitiveID"),
SP(Kind::GSInstanceID, "SV_GSInstanceID"),
SP(Kind::SampleIndex, "SV_SampleIndex"),
SP(Kind::IsFrontFace, "SV_IsFrontFace"),
SP(Kind::Coverage, "SV_Coverage"),
SP(Kind::InnerCoverage, "SV_InnerCoverage"),
SP(Kind::Target, "SV_Target"),
SP(Kind::Depth, "SV_Depth"),
SP(Kind::DepthLessEqual, "SV_DepthLessEqual"),
SP(Kind::DepthGreaterEqual, "SV_DepthGreaterEqual"),
SP(Kind::StencilRef, "SV_StencilRef"),
SP(Kind::DispatchThreadID, "SV_DispatchThreadID"),
SP(Kind::GroupID, "SV_GroupID"),
SP(Kind::GroupIndex, "SV_GroupIndex"),
SP(Kind::GroupThreadID, "SV_GroupThreadID"),
SP(Kind::TessFactor, "SV_TessFactor"),
SP(Kind::InsideTessFactor, "SV_InsideTessFactor"),
SP(Kind::ViewID, "SV_ViewID"),
SP(Kind::Barycentrics, "SV_Barycentrics"),
SP(Kind::ShadingRate, "SV_ShadingRate"),
SP(Kind::CullPrimitive, "SV_CullPrimitive"),
SP(Kind::Invalid, nullptr),
// Kind Name Component Size
SP(Kind::Arbitrary, nullptr, CompClass::Any, SizeClass::Other),
SP(Kind::VertexID, "SV_VertexID", CompClass::Uint, SizeClass::Scalar),
SP(Kind::InstanceID, "SV_InstanceID", CompClass::Uint, SizeClass::Scalar),
SP(Kind::Position, "SV_Position", CompClass::Float, SizeClass::Vec4),
SP(Kind::RenderTargetArrayIndex,"SV_RenderTargetArrayIndex",CompClass::Uint, SizeClass::Scalar),
SP(Kind::ViewPortArrayIndex, "SV_ViewportArrayIndex", CompClass::Uint, SizeClass::Scalar),
SP(Kind::ClipDistance, "SV_ClipDistance", CompClass::Float, SizeClass::Other),
SP(Kind::CullDistance, "SV_CullDistance", CompClass::Float, SizeClass::Other),
SP(Kind::OutputControlPointID, "SV_OutputControlPointID", CompClass::Uint, SizeClass::Scalar),
SP(Kind::DomainLocation, "SV_DomainLocation", CompClass::Float, SizeClass::MaxVec3),
SP(Kind::PrimitiveID, "SV_PrimitiveID", CompClass::Uint, SizeClass::Scalar),
SP(Kind::GSInstanceID, "SV_GSInstanceID", CompClass::Uint, SizeClass::Scalar),
SP(Kind::SampleIndex, "SV_SampleIndex", CompClass::Uint, SizeClass::Scalar),
SP(Kind::IsFrontFace, "SV_IsFrontFace", CompClass::Bool, SizeClass::Scalar),
SP(Kind::Coverage, "SV_Coverage", CompClass::Uint, SizeClass::Scalar),
SP(Kind::InnerCoverage, "SV_InnerCoverage", CompClass::Uint, SizeClass::Scalar),
SP(Kind::Target, "SV_Target", CompClass::Any, SizeClass::MaxVec4),
SP(Kind::Depth, "SV_Depth", CompClass::Float, SizeClass::Scalar),
SP(Kind::DepthLessEqual, "SV_DepthLessEqual", CompClass::Float, SizeClass::Scalar),
SP(Kind::DepthGreaterEqual, "SV_DepthGreaterEqual", CompClass::Float, SizeClass::Scalar),
SP(Kind::StencilRef, "SV_StencilRef", CompClass::Uint, SizeClass::Scalar),
SP(Kind::DispatchThreadID, "SV_DispatchThreadID", CompClass::Uint, SizeClass::MaxVec3),
SP(Kind::GroupID, "SV_GroupID", CompClass::Uint, SizeClass::MaxVec3),
SP(Kind::GroupIndex, "SV_GroupIndex", CompClass::Uint, SizeClass::Scalar),
SP(Kind::GroupThreadID, "SV_GroupThreadID", CompClass::Uint, SizeClass::MaxVec3),
SP(Kind::TessFactor, "SV_TessFactor", CompClass::Float, SizeClass::Other),
SP(Kind::InsideTessFactor, "SV_InsideTessFactor", CompClass::Float, SizeClass::Other),
SP(Kind::ViewID, "SV_ViewID", CompClass::Uint, SizeClass::Scalar),
SP(Kind::Barycentrics, "SV_Barycentrics", CompClass::Float, SizeClass::MaxVec3),
SP(Kind::ShadingRate, "SV_ShadingRate", CompClass::Uint, SizeClass::Scalar),
SP(Kind::CullPrimitive, "SV_CullPrimitive", CompClass::Bool, SizeClass::Scalar),
SP(Kind::Invalid, nullptr, CompClass::Any, SizeClass::Other),
};

} // namespace hlsl
1 change: 1 addition & 0 deletions tools/clang/include/clang/AST/HlslTypes.h
Original file line number Diff line number Diff line change
Expand Up @@ -396,6 +396,7 @@ void GetRowsAndColsForAny(clang::QualType type, uint32_t &rowCount,
uint32_t &colCount);
uint32_t GetElementCount(clang::QualType type);
uint32_t GetArraySize(clang::QualType type);
clang::QualType GetArrayElementType(clang::QualType type);
uint32_t GetHLSLVecSize(clang::QualType type);
void GetRowsAndCols(clang::QualType type, uint32_t &rowCount,
uint32_t &colCount);
Expand Down
10 changes: 10 additions & 0 deletions tools/clang/lib/AST/HlslTypes.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ bool IsHLSLVecType(clang::QualType type) {
}
}
}

return false;
}

Expand Down Expand Up @@ -128,6 +129,7 @@ bool IsHLSLNumericUserDefinedType(clang::QualType type) {
return false;
}


// Aggregate types are arrays and user-defined structs
bool IsHLSLAggregateType(clang::QualType type) {
type = type.getCanonicalType();
Expand Down Expand Up @@ -268,6 +270,14 @@ uint32_t GetElementCount(clang::QualType type) {
return rowCount * colCount;
}

/// <summary>Returns the element type in the specified array
/// type.</summary>
QualType GetArrayElementType(clang::QualType type) {
QualType qTy = type.getCanonicalType().getNonReferenceType();
DXASSERT_NOMSG(qTy->isArrayType() && "otherwise caller shouldn't be invoking this");
return qTy->getAsArrayTypeUnsafe()->getElementType();
}

/// <summary>Returns the number of elements in the specified array
/// type.</summary>
uint32_t GetArraySize(clang::QualType type) {
Expand Down
174 changes: 165 additions & 9 deletions tools/clang/lib/CodeGen/CGHLSLMS.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -178,11 +178,20 @@ class CGMSHLSLRuntime : public CGHLSLRuntime {

void CheckParameterAnnotation(SourceLocation SLoc,
const DxilParameterAnnotation &paramInfo,
QualType &paramTy,
bool isPatchConstantFunction);
void CheckParameterAnnotation(SourceLocation SLoc,
DxilParamInputQual paramQual,
QualType &paramTy,
llvm::StringRef semFullName,
bool isPatchConstantFunction);
void DiagnoseSemanticType(SourceLocation SLoc,
const Semantic *pSemantic, QualType &paramTy, unsigned semIndex);
bool IsAllowedSemanticType(const Semantic *pSemantic, QualType &paramTy,
unsigned semIndex);

Semantic::CompClass GetCompClass(QualType &type);
Semantic::SizeClass GetSizeClass(QualType &type);

void RemapObsoleteSemantic(DxilParameterAnnotation &paramInfo,
bool isPatchConstantFunction);
Expand Down Expand Up @@ -434,25 +443,169 @@ void CGMSHLSLRuntime::AddHLSLIntrinsicOpcodeToFunction(Function *F,
m_IntrinsicMap.emplace_back(F,opcode);
}

static clang::Type::ScalarTypeKind GetScalarElementType(QualType qTy) {
if (IsHLSLMatType(qTy))
return GetScalarElementType(GetHLSLMatElementType(qTy));

if (IsHLSLVecType(qTy))
return GetScalarElementType(GetHLSLVecElementType(qTy));

if (qTy->isArrayType())
return GetScalarElementType(GetArrayElementType(qTy));

DXASSERT_NOMSG(qTy->isScalarType());
return qTy->getScalarTypeKind();
}

static unsigned GetCompSize(QualType qty) {
if (qty->isScalarType())
return 1;
if (qty->isArrayType())
return GetCompSize(GetArrayElementType(qty));
if (IsHLSLMatType(qty))
return GetCompSize(GetHLSLMatElementType(qty));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we should support array or matrix for system values - other than under the Other designation for special cases.
No system value can be an array except for special cases handled under Other.
If fxc suports matrix types, I think that's an oversight, and should not be by-design. If the matrix had multiple rows, that would be illegal for all system values except tessfactors.

DXASSERT_NOMSG(IsHLSLVecType(qty));
return GetHLSLVecSize(qty);
}

Semantic::CompClass CGMSHLSLRuntime::GetCompClass(QualType &type) {
QualType qTy = type.getCanonicalType().getNonReferenceType();
if (!qTy->isScalarType() &&
!IsHLSLVecType(qTy) &&
!qTy->isArrayType()) {
return Semantic::CompClass::Any;
}

clang::Type::ScalarTypeKind scalarKind = GetScalarElementType(qTy);
switch (scalarKind) {
case clang::Type::ScalarTypeKind::STK_Bool:
return Semantic::CompClass::Bool;
case clang::Type::ScalarTypeKind::STK_Integral:
return Semantic::CompClass::Uint;
case clang::Type::ScalarTypeKind::STK_Floating:
return Semantic::CompClass::Float;
default:
return Semantic::CompClass::Any;
}
}

Semantic::SizeClass CGMSHLSLRuntime::GetSizeClass(QualType &type) {
QualType qTy = type.getCanonicalType().getNonReferenceType();
if (!qTy->isScalarType() &&
!IsHLSLVecType(qTy) &&
!qTy->isArrayType()) {
return Semantic::SizeClass::Other;
}

unsigned size = GetCompSize(qTy);
switch (size) {
case 1:
return Semantic::SizeClass::Scalar;
case 2:
return Semantic::SizeClass::Vec2;
case 3:
return Semantic::SizeClass::Vec3;
case 4:
return Semantic::SizeClass::Vec4;
default:
return Semantic::SizeClass::Other;
}
}

static bool IsAllowedSizeClass(Semantic::SizeClass sizeClass,
const Semantic *pSemantic,
unsigned semIndex) {
Semantic::SizeClass expSizeClass = pSemantic->GetSizeClass();
if (expSizeClass == Semantic::SizeClass::Other) {
switch (pSemantic->GetKind()) {
case Semantic::Kind::ClipDistance:
case Semantic::Kind::CullDistance:
return true;
case Semantic::Kind::TessFactor:
return (semIndex <= 3);
case Semantic::Kind::InsideTessFactor:
return (semIndex <= 1);
default:
return false;
}
}

switch (sizeClass) {
case Semantic::SizeClass::Scalar:
return true;
case Semantic::SizeClass::Vec2:
return (expSizeClass == Semantic::SizeClass::Vec2 ||
expSizeClass == Semantic::SizeClass::MaxVec2 ||
expSizeClass == Semantic::SizeClass::MaxVec3 ||
expSizeClass == Semantic::SizeClass::MaxVec4);
case Semantic::SizeClass::Vec3:
return (expSizeClass == Semantic::SizeClass::Vec3 ||
expSizeClass == Semantic::SizeClass::MaxVec3 ||
expSizeClass == Semantic::SizeClass::MaxVec4);
case Semantic::SizeClass::Vec4:
return (expSizeClass == Semantic::SizeClass::Vec4 ||
expSizeClass == Semantic::SizeClass::MaxVec4);
default:
return false;
}
}

static bool IsAllowedCompClass(Semantic::CompClass compClass,
const Semantic *pSemantic) {
Semantic::CompClass expCompClass = pSemantic->GetCompClass();
if (expCompClass == Semantic::CompClass::Any)
return true;
return (expCompClass == compClass);
}

bool CGMSHLSLRuntime::IsAllowedSemanticType(const Semantic *pSemantic,
QualType &paramTy,
unsigned semIndex) {
QualType qTy = paramTy.getCanonicalType().getNonReferenceType();
if (qTy->isStructureType()) {
const RecordType *rTy = cast<RecordType>(qTy);
bool isAllowed = true;
for (FieldDecl *fieldDecl : rTy->getDecl()->fields()) {
QualType fieldTy = fieldDecl->getType().getCanonicalType().getNonReferenceType();
isAllowed = isAllowed && IsAllowedSemanticType(pSemantic, fieldTy, semIndex);
}
return isAllowed;
}
return IsAllowedCompClass(GetCompClass(qTy), pSemantic) &&
IsAllowedSizeClass(GetSizeClass(qTy), pSemantic, semIndex);
}

void CGMSHLSLRuntime::DiagnoseSemanticType(SourceLocation SLoc,
const Semantic *pSemantic,
QualType &paramTy,
unsigned semIndex) {
if (!IsAllowedSemanticType(pSemantic, paramTy, semIndex)) {
DiagnosticsEngine &Diags = CGM.getDiags();
unsigned DiagID = Diags.getCustomDiagID(
DiagnosticsEngine::Error, "invalid type used for '%0' input semantics");
Diags.Report(SLoc, DiagID) << pSemantic->GetName();
}
}

void CGMSHLSLRuntime::CheckParameterAnnotation(
SourceLocation SLoc, const DxilParameterAnnotation &paramInfo,
bool isPatchConstantFunction) {
SourceLocation SLoc, const DxilParameterAnnotation &paramInfo,
QualType &paramTy, bool isPatchConstantFunction) {
if (!paramInfo.HasSemanticString()) {
return;
}
llvm::StringRef semFullName = paramInfo.GetSemanticStringRef();
DxilParamInputQual paramQual = paramInfo.GetParamInputQual();
if (paramQual == DxilParamInputQual::Inout) {
CheckParameterAnnotation(SLoc, DxilParamInputQual::In, semFullName, isPatchConstantFunction);
CheckParameterAnnotation(SLoc, DxilParamInputQual::Out, semFullName, isPatchConstantFunction);
CheckParameterAnnotation(SLoc, DxilParamInputQual::In, paramTy, semFullName, isPatchConstantFunction);
CheckParameterAnnotation(SLoc, DxilParamInputQual::Out, paramTy, semFullName, isPatchConstantFunction);
return;
}
CheckParameterAnnotation(SLoc, paramQual, semFullName, isPatchConstantFunction);
CheckParameterAnnotation(SLoc, paramQual, paramTy, semFullName, isPatchConstantFunction);
}

void CGMSHLSLRuntime::CheckParameterAnnotation(
SourceLocation SLoc, DxilParamInputQual paramQual, llvm::StringRef semFullName,
bool isPatchConstantFunction) {
SourceLocation SLoc, DxilParamInputQual paramQual, QualType &paramTy,
llvm::StringRef semFullName, bool isPatchConstantFunction) {
const ShaderModel *SM = m_pHLModule->GetShaderModel();

DXIL::SigPointKind sigPoint = SigPointFromInputQual(
Expand All @@ -470,6 +623,9 @@ void CGMSHLSLRuntime::CheckParameterAnnotation(
Diags.getCustomDiagID(DiagnosticsEngine::Error, "invalid semantic '%0' for %1 %2.%3");
Diags.Report(SLoc, DiagID) << semName << SM->GetKindName() << SM->GetMajor() << SM->GetMinor();
}

if (pSemantic->GetName())
DiagnoseSemanticType(SLoc, pSemantic, paramTy, semIndex);
}

SourceLocation
Expand Down Expand Up @@ -1624,7 +1780,7 @@ void CGMSHLSLRuntime::AddHLSLFunctionInfo(Function *F, const FunctionDecl *FD) {
RemapObsoleteSemantic(retTyAnnotation, /*isPatchConstantFunction*/ false);
}
CheckParameterAnnotation(retTySemanticLoc, retTyAnnotation,
/*isPatchConstantFunction*/ false);
retTy, /*isPatchConstantFunction*/ false);
}
if (isRay && !retTy->isVoidType()) {
Diags.Report(FD->getLocation(), Diags.getCustomDiagID(
Expand Down Expand Up @@ -2070,7 +2226,7 @@ void CGMSHLSLRuntime::AddHLSLFunctionInfo(Function *F, const FunctionDecl *FD) {
RemapObsoleteSemantic(paramAnnotation, /*isPatchConstantFunction*/ false);
}
CheckParameterAnnotation(paramSemanticLoc, paramAnnotation,
/*isPatchConstantFunction*/ false);
fieldTy, /*isPatchConstantFunction*/ false);
}
}

Expand Down
Loading