Skip to content

Commit

Permalink
address tex feedback, add test to check if prog ver / DMod ver differ…
Browse files Browse the repository at this point in the history
…s, remove test ll file
  • Loading branch information
bob80905 committed Jan 23, 2025
1 parent f4cf764 commit dafab82
Show file tree
Hide file tree
Showing 10 changed files with 106 additions and 76 deletions.
5 changes: 2 additions & 3 deletions include/dxc/DXIL/DxilShaderModel.h
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ class ShaderModel {
// Major/Minor version of highest released shader model
// clang-format off
// Python lines need to be not formatted.
/* <py::lines('VALRULE-TEXT')>hctdb_instrhelp.get_highest_recognized_shader_model()</py>*/
/* <py::lines('VALRULE-TEXT')>hctdb_instrhelp.get_highest_shader_model()</py>*/
// clang-format on
// VALRULE-TEXT:BEGIN
static const unsigned kHighestMajor = 6;
Expand Down Expand Up @@ -95,8 +95,7 @@ class ShaderModel {
static const ShaderModel *GetByName(llvm::StringRef Name);
static const char *GetKindName(Kind kind);
static bool IsPreReleaseShaderModel(int Major, int Minor);
static const ShaderModel *GetPreReleaseShaderModel(llvm::StringRef name);
static const Kind GetKindFromName(llvm::StringRef Name);
static Kind GetKindFromName(llvm::StringRef Name);
static DXIL::ShaderKind KindFromFullName(llvm::StringRef Name);
static const llvm::StringRef FullNameFromKind(DXIL::ShaderKind sk);
static const char *GetNodeLaunchTypeName(DXIL::NodeLaunchType launchTy);
Expand Down
3 changes: 3 additions & 0 deletions include/dxc/DxilContainer/DxilContainer.h
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,9 @@ struct DxilContainerHash {
uint8_t Digest[DxilContainerHashSize];
};

static const DxilContainerHash PreviewByPassHash = {2, 2, 2, 2, 2, 2, 2, 2,
2, 2, 2, 2, 2, 2, 2, 2};

enum class DxilShaderHashFlags : uint32_t {
None = 0, // No flags defined.
IncludesSource = 1, // This flag indicates that the shader hash was computed
Expand Down
5 changes: 5 additions & 0 deletions include/dxc/Support/ErrorCodes.h
Original file line number Diff line number Diff line change
Expand Up @@ -153,3 +153,8 @@
// 0X80AA001E - External validator (DXIL.dll) required, and missing.
#define DXC_E_VALIDATOR_MISSING \
DXC_MAKE_HRESULT(DXC_SEVERITY_ERROR, FACILITY_DXC, (0x001E))

// 0X80AA001F - DXIL container Program Version mismatches Dxil module shader
// model
#define DXC_E_INCORRECT_PROGRAM_VERSION \
DXC_MAKE_HRESULT(DXC_SEVERITY_ERROR, FACILITY_DXC, (0x001F))
2 changes: 1 addition & 1 deletion lib/DXIL/DxilShaderModel.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,7 @@ bool ShaderModel::IsPreReleaseShaderModel(int major, int minor) {
return false;
}

const ShaderModel::Kind ShaderModel::GetKindFromName(llvm::StringRef Name) {
ShaderModel::Kind ShaderModel::GetKindFromName(llvm::StringRef Name) {
Kind kind;
if (Name.empty()) {
return Kind::Invalid;
Expand Down
22 changes: 22 additions & 0 deletions lib/DxilValidation/DxilContainerValidation.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1275,6 +1275,28 @@ static HRESULT ValidateLoadModuleFromContainer(
}
}

// Verify that the ProgramHeader in the container contains a program version
// that matches the DxilModule's shader model version
const DxilProgramHeader *pProgramHeader =
reinterpret_cast<const DxilProgramHeader *>(GetDxilPartData(pPart));
if (pProgramHeader) {
int PV = pProgramHeader->ProgramVersion;
int major = (PV >> 4) & 0xF; // Extract the major version (next 4 bits)
int minor = PV & 0xF; // Extract the minor version (lowest 4 bits)
DxilModule *pDxilModule = DxilModule::TryGetDxilModule(&*pModule);

ValidationContext ValCtx(*pModule, &*pDebugModule, *pDxilModule);
int moduleMajor = ValCtx.DxilMod.GetShaderModel()->GetMajor();
int moduleMinor = ValCtx.DxilMod.GetShaderModel()->GetMinor();
if (moduleMajor != major || moduleMinor != minor) {
ValCtx.EmitFormatError(ValidationRule::SmProgramVersion,
{std::to_string(major), std::to_string(minor),
std::to_string(moduleMajor),
std::to_string(moduleMinor)});
return DXC_E_INCORRECT_PROGRAM_VERSION;
}
}

return S_OK;
}

Expand Down
39 changes: 0 additions & 39 deletions tools/clang/test/HLSLFileCheck/validation/prerelase-hash.ll

This file was deleted.

5 changes: 3 additions & 2 deletions tools/clang/tools/dxcvalidator/dxcvalidator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,8 @@ using namespace hlsl;
static void HashAndUpdate(DxilContainerHeader *Container, bool isPreRelease) {
if (isPreRelease) {
// If preview bypass is enabled, use the preview hash.
UINT previewHash[4] = {0x02020202, 0x02020202, 0x02020202, 0x02020202};
memcpy(Container->Hash.Digest, previewHash, sizeof(previewHash));
memcpy(Container->Hash.Digest, PreviewByPassHash.Digest,
sizeof(PreviewByPassHash.Digest));
return;
}
// Compute hash and update stored hash.
Expand All @@ -61,6 +61,7 @@ static void HashAndUpdateOrCopy(uint32_t Flags, IDxcBlob *Shader,
const DxilProgramHeader *ProgramHeader =
GetDxilProgramHeader(DxilContainer, DFCC_DXIL);

// ProgramHeader may be null here, when hasing a root signature container
if (ProgramHeader) {
int PV = ProgramHeader->ProgramVersion;
int major = (PV >> 4) & 0xF; // Extract the major version (next 4 bits)
Expand Down
48 changes: 42 additions & 6 deletions tools/clang/unittests/HLSL/ValidationTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4237,15 +4237,51 @@ TEST_F(ValidationTest, ValidatePreviewBypassHash) {

pHeader = (hlsl::DxilContainerHeader *)pProgram->GetBufferPointer();

// Validate the hash.
BYTE PreviewBypassHash[DxilContainerHashSize] = {2, 2, 2, 2, 2, 2, 2, 2,
2, 2, 2, 2, 2, 2, 2, 2};

// Should be equal, this proves the hash is set to the preview bypass hash
// when a prerelease version is used
VERIFY_ARE_EQUAL(memcmp(PreviewBypassHash, pHeader->Hash.Digest,
sizeof(PreviewBypassHash)),
VERIFY_ARE_EQUAL(memcmp(&hlsl::PreviewByPassHash, pHeader->Hash.Digest,
sizeof(hlsl::PreviewByPassHash)),
0);

// test that when the program version differs from the dxil module shader
// model version, the validator fails
DxilPartHeader *pPart = GetDxilPartByType(pHeader, DxilFourCC::DFCC_DXIL);

DxilProgramHeader *pMutableProgramHeader =
reinterpret_cast<DxilProgramHeader *>(GetDxilPartData(pPart));
int oldMajor = 0;
int oldMinor = 0;
int newMajor = 0;
int newMinor = 0;
VERIFY_IS_NOT_NULL(pMutableProgramHeader);
uint32_t &PV = pMutableProgramHeader->ProgramVersion;
oldMajor = (PV >> 4) & 0xF; // Extract the major version (next 4 bits)
oldMinor = PV & 0xF; // Extract the minor version (lowest 4 bits)

// flip the bit of the shader model version minor
PV ^= 1;

newMajor = (PV >> 4) & 0xF; // Extract the major version (next 4 bits)
newMinor = PV & 0xF; // Extract the minor version (lowest 4 bits)

// now test that the validation fails
pResult.Release();
VERIFY_SUCCEEDED(pValidator->Validate(pProgram, Flags, &pResult));
VERIFY_IS_NOT_NULL(pResult);
pResult->GetStatus(&status);

// expect validation to fail
VERIFY_FAILED(status);
// validation succeeded prior, so by inference we know that oldMajor /
// oldMinor were the old dxil module shader model versions
char buffer[100];
std::sprintf(buffer,
"error: Program Version is %d.%d but Dxil Module shader model "
"version is %d.%d.\nValidation failed.\n",
newMajor, newMinor, oldMajor, oldMinor);
std::string formattedString = buffer;

CheckOperationResultMsgs(pResult, {buffer}, false, false);
}

TEST_F(ValidationTest, ValidateVersionNotAllowed) {
Expand Down
5 changes: 5 additions & 0 deletions utils/hct/hctdb.py
Original file line number Diff line number Diff line change
Expand Up @@ -7500,6 +7500,11 @@ def build_valrules(self):
"Target shader model requires specific Dxil Version",
"Shader model requires Dxil Version %0.%1.",
)
self.add_valrule_msg(
"Sm.ProgramVersion",
"Program Version in Dxil Container does not match Dxil Module shader model version",
"Program Version is %0.%1 but Dxil Module shader model version is %2.%3.",
)
self.add_valrule_msg(
"Sm.Opcode",
"Opcode must be defined in target shader model",
Expand Down
48 changes: 23 additions & 25 deletions utils/hct/hctdb_instrhelp.py
Original file line number Diff line number Diff line change
Expand Up @@ -1537,23 +1537,21 @@ def get_interpretation_table():
gen = db_sigpoint_gen(db)
return run_with_stdout(lambda: gen.print_interpretation_table())

# highest recognized is different than highest released,
# since there can be prerelease versions that are higher
# highest minor is different than highest released minor,
# since there can be pre-release versions that are higher
# than the last released version
highest_recognized_major = 6
highest_recognized_minor = 9

highest_major = 6
highest_minor = 9
highest_shader_models = {4: 1, 5: 1, 6: highest_minor}

# fetch the last released version from latest-released.json
json_path = os.path.dirname(os.path.dirname(__file__)) + "/version/latest-release.json"
with open(json_path, 'r') as file:
with open(json_path, "r") as file:
json_data = json.load(file)

highest_released_minor = int(json_data["version"]["minor"])


highest_shader_models = {4: 1, 5: 1, 6: highest_recognized_minor}

def getShaderModels():
shader_models = []
for major, minor in highest_shader_models.items():
Expand All @@ -1566,35 +1564,35 @@ def getShaderModels():
def get_highest_released_shader_model():
result = """static const unsigned kHighestReleasedMajor = %d;
static const unsigned kHighestReleasedMinor = %d;""" % (
highest_recognized_major,
highest_major,
highest_released_minor,
)
return result

def get_highest_recognized_shader_model():
def get_highest_shader_model():
result = """static const unsigned kHighestMajor = %d;
static const unsigned kHighestMinor = %d;""" % (
highest_recognized_major,
highest_recognized_minor,
highest_major,
highest_minor,
)
return result

def get_dxil_version_minor():
return "const unsigned kDxilMinor = %d;" % highest_recognized_minor
return "const unsigned kDxilMinor = %d;" % highest_minor


def get_dxil_version_minor_int():
return highest_recognized_minor
return highest_minor


def get_is_shader_model_plus():
result = ""

for i in range(0, highest_recognized_minor + 1):
for i in range(0, highest_minor + 1):
result += "bool IsSM%d%dPlus() const { return IsSMAtLeast(%d, %d); }\n" % (
highest_recognized_major,
highest_major,
i,
highest_recognized_major,
highest_major,
i,
)
return result
Expand Down Expand Up @@ -1798,7 +1796,7 @@ def get_validation_version():
*pMajor = 1;
*pMinor = %d;
"""
% highest_recognized_minor
% highest_minor
)
return result

Expand All @@ -1810,7 +1808,7 @@ def get_target_profiles():
profiles = getShaderProfiles()
shader_models = getShaderModels()

base_sm = "%d_0" % highest_recognized_major
base_sm = "%d_0" % highest_major
for profile, min_sm in profiles:
for shader_model in shader_models:
if base_sm > shader_model:
Expand All @@ -1826,7 +1824,7 @@ def get_target_profiles():

def get_min_validator_version():
result = ""
for i in range(0, highest_recognized_minor + 1):
for i in range(0, highest_minor + 1):
result += "case %d:\n" % i
result += " ValMinor = %d;\n" % i
result += " break;\n"
Expand All @@ -1835,12 +1833,12 @@ def get_min_validator_version():

def get_dxil_version():
result = ""
for i in range(0, highest_recognized_minor + 1):
for i in range(0, highest_minor + 1):
result += "case %d:\n" % i
result += " DxilMinor = %d;\n" % i
result += " break;\n"
result += "case kOfflineMinor: // Always update this to highest dxil version\n"
result += " DxilMinor = %d;\n" % highest_recognized_minor
result += " DxilMinor = %d;\n" % highest_minor
result += " break;\n"
return result

Expand All @@ -1859,9 +1857,9 @@ def get_shader_model_get():

def get_shader_model_by_name():
result = ""
for i in range(2, highest_recognized_minor + 1):
for i in range(2, highest_minor + 1):
result += "case '%d':\n" % i
result += " if (Major == %d) {\n" % highest_recognized_major
result += " if (Major == %d) {\n" % highest_major
result += " Minor = %d;\n" % i
result += " break;\n"
result += " }\n"
Expand All @@ -1872,7 +1870,7 @@ def get_shader_model_by_name():

def get_is_valid_for_dxil():
result = ""
for i in range(0, highest_recognized_minor + 1):
for i in range(0, highest_minor + 1):
result += "case %d:\n" % i
return result

Expand Down

0 comments on commit dafab82

Please sign in to comment.