Skip to content

Commit

Permalink
address feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
@bob80905
bob80905 committed Dec 15, 2023
1 parent 3ea81db commit 7b97fad
Show file tree
Hide file tree
Showing 6 changed files with 128 additions and 129 deletions.
2 changes: 1 addition & 1 deletion docs/DXIL.rst
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3071,7 +3071,6 @@ INSTR.FAILTORESLOVETGSMPOINTER TGSM pointers must originate from an u
INSTR.HANDLENOTFROMCREATEHANDLE Resource handle should returned by createHandle.
INSTR.IMMBIASFORSAMPLEB bias amount for sample_b must be in the range [%0,%1], but %2 was specified as an immediate.
INSTR.INBOUNDSACCESS Access to out-of-bounds memory is disallowed.
INSTR.INVALIDSVINFUNCTION An SV value was used on a parameter to a node shader function that isn't compatible.
INSTR.MINPRECISIONNOTPRECISE Instructions marked precise may not refer to minprecision values.
INSTR.MINPRECISONBITCAST Bitcast on minprecison types is not allowed.
INSTR.MIPLEVELFORGETDIMENSION Use mip level on buffer when GetDimensions.
Expand Down Expand Up @@ -3125,6 +3124,7 @@ INSTR.SAMPLERMODEFORSAMPLEC sample_c_*/gather_c instructions requi
INSTR.SIGNATUREOPERATIONNOTINENTRY Dxil operation for input output signature must be in entryPoints.
INSTR.STATUS Resource status should only be used by CheckAccessFullyMapped.
INSTR.STRUCTBITCAST Bitcast on struct types is not allowed.
INSTR.SVCONFLICTINGLAUNCHMODE Input system values are compatible with node shader launch mode.
INSTR.TEXTUREOFFSET offset texture instructions must take offset which can resolve to integer literal in the range -8 to 7.
INSTR.TGSMRACECOND Race condition writing to shared memory detected, consider making this write conditional.
INSTR.UNDEFINEDVALUEFORUAVSTORE Assignment of undefined values to UAV.
Expand Down
243 changes: 124 additions & 119 deletions lib/HLSL/DxilValidation.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2134,6 +2134,24 @@ static void ValidateBarrierFlagArg(ValidationContext &ValCtx, CallInst *CI,
}
}

std::string GetLaunchTypeStr(DXIL::NodeLaunchType LT) {
switch (LT) {
case DXIL::NodeLaunchType::Broadcasting:
return "Broadcasting";
case DXIL::NodeLaunchType::Coalescing:
return "Coalescing";
case DXIL::NodeLaunchType::Thread:
return "Thread";
case DXIL::NodeLaunchType::Invalid:
return "Invalid";
case DXIL::NodeLaunchType::LastEntry:
return "LastEntry";

default:
return "Unknown";
}
}

static void ValidateDxilOperationCallInProfile(CallInst *CI,
DXIL::OpCode opcode,
const ShaderModel *pSM,
Expand All @@ -2156,6 +2174,15 @@ static void ValidateDxilOperationCallInProfile(CallInst *CI,
// Is called from a library function
bool isLibFunc = shaderKind == DXIL::ShaderKind::Library;

// get the node launch type
DxilModule &DM = ValCtx.DxilMod;
DXIL::NodeLaunchType NLT = DXIL::NodeLaunchType::Invalid;

if (DM.HasDxilEntryProps(F)) {
DxilEntryProps &entryProps = DM.GetDxilEntryProps(F);
NLT = entryProps.props.Node.LaunchType;
}

ValidateHandleArgs(CI, opcode, ValCtx);

switch (opcode) {
Expand Down Expand Up @@ -2346,80 +2373,62 @@ static void ValidateDxilOperationCallInProfile(CallInst *CI,
{"CreateHandle", "Shader model 6.5 and below"});
}
} break;

// this intrinsic is generated when the SV_DispatchThreadID system value is used
case DXIL::OpCode::ThreadId: {
// get the node launch type
DxilModule &DM = ValCtx.DxilMod;
if (!DM.HasDxilEntryProps(F)) {

case DXIL::OpCode::ThreadId: { // SV_DispatchThreadID
if (shaderKind != DXIL::ShaderKind::Node) {
break;
}

DxilEntryProps &entryProps = DM.GetDxilEntryProps(F);
DXIL::NodeLaunchType NLT = entryProps.props.Node.LaunchType;
if (NLT == DXIL::NodeLaunchType::Thread || NLT == DXIL::NodeLaunchType::Coalescing) {
ValCtx.EmitInstrFormatError(CI, ValidationRule::InstrInvalidSVInFunction,
{"ThreadId", NLT ==
DXIL::NodeLaunchType::Thread
? "Thread"
: "Coalescing"});
}
} break;
// this intrinsic is generated when the SV_GroupId
// system value is used
if (NLT == DXIL::NodeLaunchType::Broadcasting) {
// good
} else {
ValCtx.EmitInstrFormatError(
CI, ValidationRule::InstrSVConflictingLaunchMode,
{"ThreadId", "SV_DispatchThreadID", GetLaunchTypeStr(NLT)});
}
} break;

case DXIL::OpCode::GroupId: {
// get the node launch type
DxilModule &DM = ValCtx.DxilMod;
if (!DM.HasDxilEntryProps(F)) {
case DXIL::OpCode::GroupId: { // SV_GroupId
if (shaderKind != DXIL::ShaderKind::Node) {
break;
}

DxilEntryProps &entryProps = DM.GetDxilEntryProps(F);
DXIL::NodeLaunchType NLT = entryProps.props.Node.LaunchType;
if (NLT == DXIL::NodeLaunchType::Thread || NLT == DXIL::NodeLaunchType::Coalescing) {
ValCtx.EmitInstrFormatError(CI, ValidationRule::InstrInvalidSVInFunction,
{"GroupId", NLT ==
DXIL::NodeLaunchType::Thread
? "Thread"
: "Coalescing"});
}
}
break;

// this intrinsic is generated when the SV_GroupThreadID
// system value is used
case DXIL::OpCode::ThreadIdInGroup: {
// get the node launch type
DxilModule &DM = ValCtx.DxilMod;
if (!DM.HasDxilEntryProps(F)) {
break;
if (NLT == DXIL::NodeLaunchType::Broadcasting) {
// good
} else {
ValCtx.EmitInstrFormatError(
CI, ValidationRule::InstrSVConflictingLaunchMode,
{"GroupId", "SV_GroupId", GetLaunchTypeStr(NLT)});
}
} break;

case DXIL::OpCode::ThreadIdInGroup: { // SV_GroupThreadID
if (shaderKind != DXIL::ShaderKind::Node) {
break;
}

DxilEntryProps &entryProps = DM.GetDxilEntryProps(F);
DXIL::NodeLaunchType NLT = entryProps.props.Node.LaunchType;
if (NLT == DXIL::NodeLaunchType::Thread) {
ValCtx.EmitInstrFormatError(
CI, ValidationRule::InstrInvalidSVInFunction,
{"ThreadIdInGroup", "Thread"});
if (NLT == DXIL::NodeLaunchType::Broadcasting ||
NLT == DXIL::NodeLaunchType::Coalescing) {
// good
} else {
ValCtx.EmitInstrFormatError(
CI, ValidationRule::InstrSVConflictingLaunchMode,
{"ThreadIdInGroup", "SV_GroupThreadID", GetLaunchTypeStr(NLT)});
}
} break;

// this intrinsic is generated when the SV_GroupIndex
// system value is used
case DXIL::OpCode::FlattenedThreadIdInGroup: {
// get the node launch type
DxilModule &DM = ValCtx.DxilMod;
if (!DM.HasDxilEntryProps(F)) {
break;

case DXIL::OpCode::FlattenedThreadIdInGroup: { // SV_GroupIndex
if (shaderKind != DXIL::ShaderKind::Node) {
break;
}

DxilEntryProps &entryProps = DM.GetDxilEntryProps(F);
DXIL::NodeLaunchType NLT = entryProps.props.Node.LaunchType;
if (NLT == DXIL::NodeLaunchType::Thread) {
ValCtx.EmitInstrFormatError(
CI, ValidationRule::InstrInvalidSVInFunction,
{"FlattenedThreadIdInGroup", "Thread"});
if (NLT == DXIL::NodeLaunchType::Broadcasting ||
NLT == DXIL::NodeLaunchType::Coalescing) {
// good
} else {
ValCtx.EmitInstrFormatError(
CI, ValidationRule::InstrSVConflictingLaunchMode,
{"FlattenedThreadIdInGroup", "SV_GroupIndex", GetLaunchTypeStr(NLT)});
}
} break;
default:
Expand Down Expand Up @@ -3539,62 +3548,6 @@ static void ValidateNodeInputRecord(Function *F, ValidationContext &ValCtx) {
}
}

static void ValidateFunctionArgs(Function& F, unsigned numUDTShaderArgs, DXIL::ShaderKind shaderKind, ValidationContext& ValCtx) {

auto ArgFormatError = [&](Function &F, Argument &arg, ValidationRule rule) {
if (arg.hasName())
ValCtx.EmitFnFormatError(&F, rule, {arg.getName().str(), F.getName()});
else
ValCtx.EmitFnFormatError(&F, rule,
{std::to_string(arg.getArgNo()), F.getName()});
};

unsigned numArgs = 0;
for (auto &arg : F.args()) {
Type *argTy = arg.getType();
if (argTy->isPointerTy())
argTy = argTy->getPointerElementType();

numArgs++;
if (numUDTShaderArgs) {
if (arg.getArgNo() >= numUDTShaderArgs) {
ArgFormatError(F, arg, ValidationRule::DeclExtraArgs);
} else if (!argTy->isStructTy()) {
switch (shaderKind) {
case DXIL::ShaderKind::Callable:
ArgFormatError(F, arg, ValidationRule::DeclParamStruct);
break;
default:
ArgFormatError(F, arg,
arg.getArgNo() == 0 ? ValidationRule::DeclPayloadStruct
: ValidationRule::DeclAttrStruct);
}
}
continue;
}

while (argTy->isArrayTy()) {
argTy = argTy->getArrayElementType();
}

if (argTy->isStructTy() && !ValCtx.isLibProfile) {
ArgFormatError(F, arg, ValidationRule::DeclFnFlattenParam);
break;
}
}

if (numArgs < numUDTShaderArgs && shaderKind != DXIL::ShaderKind::Node) {
StringRef argType[2] = {
shaderKind == DXIL::ShaderKind::Callable ? "params" : "payload",
"attributes"};
for (unsigned i = numArgs; i < numUDTShaderArgs; i++) {
ValCtx.EmitFnFormatError(
&F, ValidationRule::DeclShaderMissingArg,
{ShaderModel::GetKindName(shaderKind), F.getName(), argType[i]});
}
}
}

static void ValidateFunction(Function &F, ValidationContext &ValCtx) {
if (F.isDeclaration()) {
ValidateExternalFunction(&F, ValCtx);
Expand Down Expand Up @@ -3626,7 +3579,7 @@ static void ValidateFunction(Function &F, ValidationContext &ValCtx) {
}
}
break;
}
}
default:
break;
}
Expand All @@ -3645,7 +3598,59 @@ static void ValidateFunction(Function &F, ValidationContext &ValCtx) {
{F.getName()});

// Validate parameter type.
ValidateFunctionArgs(F, numUDTShaderArgs, shaderKind, ValCtx);
auto ArgFormatError = [&](Function &F, Argument &arg, ValidationRule rule) {
if (arg.hasName())
ValCtx.EmitFnFormatError(&F, rule, {arg.getName().str(), F.getName()});
else
ValCtx.EmitFnFormatError(&F, rule,
{std::to_string(arg.getArgNo()), F.getName()});
};

unsigned numArgs = 0;
for (auto &arg : F.args()) {
Type *argTy = arg.getType();
if (argTy->isPointerTy())
argTy = argTy->getPointerElementType();

numArgs++;
if (numUDTShaderArgs) {
if (arg.getArgNo() >= numUDTShaderArgs) {
ArgFormatError(F, arg, ValidationRule::DeclExtraArgs);
} else if (!argTy->isStructTy()) {
switch (shaderKind) {
case DXIL::ShaderKind::Callable:
ArgFormatError(F, arg, ValidationRule::DeclParamStruct);
break;
default:
ArgFormatError(F, arg,
arg.getArgNo() == 0
? ValidationRule::DeclPayloadStruct
: ValidationRule::DeclAttrStruct);
}
}
continue;
}

while (argTy->isArrayTy()) {
argTy = argTy->getArrayElementType();
}

if (argTy->isStructTy() && !ValCtx.isLibProfile) {
ArgFormatError(F, arg, ValidationRule::DeclFnFlattenParam);
break;
}
}

if (numArgs < numUDTShaderArgs && shaderKind != DXIL::ShaderKind::Node) {
StringRef argType[2] = {
shaderKind == DXIL::ShaderKind::Callable ? "params" : "payload",
"attributes"};
for (unsigned i = numArgs; i < numUDTShaderArgs; i++) {
ValCtx.EmitFnFormatError(
&F, ValidationRule::DeclShaderMissingArg,
{ShaderModel::GetKindName(shaderKind), F.getName(), argType[i]});
}
}

if (ValCtx.DxilMod.HasDxilFunctionProps(&F) &&
ValCtx.DxilMod.GetDxilFunctionProps(&F).IsNode()) {
Expand Down
2 changes: 0 additions & 2 deletions tools/clang/test/HLSLFileCheck/dxil/intrinsics/launch_types_broadcast.ll
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,5 +46,3 @@ declare i32 @dx.op.threadIdInGroup.i32(i32, i32) #0

; Function Attrs: nounwind readnone
declare i32 @dx.op.flattenedThreadIdInGroup.i32(i32) #0

; this file comes from compiling launch_types.hlsl with:
2 changes: 0 additions & 2 deletions tools/clang/test/HLSLFileCheck/dxil/intrinsics/launch_types_coalescing.ll
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,5 +49,3 @@ declare i32 @dx.op.threadIdInGroup.i32(i32, i32) #0

; Function Attrs: nounwind readnone
declare i32 @dx.op.flattenedThreadIdInGroup.i32(i32) #0

; this file comes from compiling launch_types.hlsl with:
2 changes: 0 additions & 2 deletions tools/clang/test/HLSLFileCheck/dxil/intrinsics/launch_types_thread.ll
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,5 +53,3 @@ declare i32 @dx.op.threadIdInGroup.i32(i32, i32) #0

; Function Attrs: nounwind readnone
declare i32 @dx.op.flattenedThreadIdInGroup.i32(i32) #0

; this file comes from compiling launch_types.hlsl with:
6 changes: 3 additions & 3 deletions utils/hct/hctdb.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7553,9 +7553,9 @@ def build_valrules(self):
"Instr.AtomicIntrinNonUAV", "Non-UAV destination to atomic intrinsic."
)
self.add_valrule_msg(
"Instr.InvalidSVInFunction",
"An SV value was used on a parameter to a node shader function that isn't compatible.",
"Call to DXIL intrinsic %0 is not allowed in node shader launch type %1"
"Instr.SVConflictingLaunchMode",
"Input system values are compatible with node shader launch mode.",
"Call to DXIL intrinsic %0 (%1) is not allowed in node shader launch type %2"
)
self.add_valrule("Instr.AtomicConst", "Constant destination to atomic.")

Expand Down

0 comments on commit 7b97fad

Please sign in to comment.