Skip to content

[HLSL Options] Remove select-validator option #7423

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.

Sign up for GitHub

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

Merged
bob80905 merged 4 commits into from
May 8, 2025
Merged

[HLSL Options] Remove select-validator option #7423

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
3a4cf10
remove select-validator option
bob80905 May 2, 2025
e87888d
add test, remove option
bob80905 May 2, 2025
2a31dcb
clean up test
bob80905 May 8, 2025
8d10c0b
move test file
bob80905 May 8, 2025
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
2 changes: 0 additions & 2 deletions include/dxc/Support/HLSLOptions.td
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -317,8 +317,6 @@ def print_before_all : Flag<["-", "/"], "print-before-all">, Group<hlslcomp_Grou
HelpText<"Print LLVM IR before each pass.">;
def print_before : Separate<["-", "/"], "print-before">, Group<hlslcomp_Group>, Flags<[CoreOption, HelpHidden]>,
HelpText<"Print LLVM IR before a specific pass. May be specificied multiple times.">;
def select_validator : Separate<["-", "/"], "select-validator">, Group<hlslcomp_Group>, Flags<[CoreOption, HelpHidden]>,
HelpText<"Select validator: auto: (default) use DXIL.dll if found, otherwise use internal; internal: internal non-signing validator; external: use DXIL.dll if found, otherwise fail compilation.">;
def print_after_all : Flag<["-", "/"], "print-after-all">, Group<hlslcomp_Group>, Flags<[CoreOption, HelpHidden]>,
HelpText<"Print LLVM IR after each pass.">;
def print_after : Separate<["-", "/"], "print-after">, Group<hlslcomp_Group>, Flags<[CoreOption, HelpHidden]>,
Expand Down
14 changes: 0 additions & 14 deletions lib/DxcSupport/HLSLOptions.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1033,20 +1033,6 @@ int ReadDxcOpts(const OptTable *optionTable, unsigned flagsToInclude,
opts.ValVerMinor = (unsigned long)minor64;
}

llvm::StringRef valSelectStr = Args.getLastArgValue(OPT_select_validator);
if (!valSelectStr.empty()) {
opts.SelectValidator = llvm::StringSwitch<ValidatorSelection>(valSelectStr)
.Case("auto", ValidatorSelection::Auto)
.Case("internal", ValidatorSelection::Internal)
.Case("external", ValidatorSelection::External)
.Default(ValidatorSelection::Invalid);
if (opts.SelectValidator == ValidatorSelection::Invalid) {
errors << "Unsupported value '" << valSelectStr
<< "for -select-validator option.";
return 1;
}
}

if (opts.IsLibraryProfile() && Minor == 0xF) {
if (opts.ValVerMajor != UINT_MAX && opts.ValVerMajor != 0) {
errors << "Offline library profile cannot be used with non-zero "
Expand Down
6 changes: 3 additions & 3 deletions tools/clang/test/HLSLFileCheck/hlsl/types/struct/struct-annotations.hlsl
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
// RUN: %dxc -T ps_6_8 -E main -Qkeep_reflect_in_dxil -select-validator internal %s | FileCheck -check-prefix=CHECK68 %s
// RUN: %dxc -T ps_6_7 -E main -Qkeep_reflect_in_dxil -select-validator internal %s | FileCheck -check-prefix=CHECK67 %s
// RUN: %dxc -T ps_6_8 -E main -Qkeep_reflect_in_dxil %s | FileCheck -check-prefix=CHECK68 %s
// RUN: %dxc -T ps_6_7 -E main -Qkeep_reflect_in_dxil %s | FileCheck -check-prefix=CHECK67 %s

// Make sure the vector is annotated with vector size (DXIL 1.8 and higher),
// matrix is annotated with matrix size and orientation, and scalar does not
Expand Down Expand Up @@ -47,4 +47,4 @@ StructuredBuffer<MyStruct> g_myStruct;
float main() : SV_Target
{
return g_myStruct[0].vec.x + g_myStruct[0].vec.y;
}
}
2 changes: 1 addition & 1 deletion tools/clang/test/HLSLFileCheck/infra/auto-dxilver.hlsl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
// Catch any other unexpected error cases.
// CHECK-NOT: error

// RUN: %dxc -T vs_6_8 -select-validator internal %s | FileCheck %s
// RUN: %dxc -T vs_6_8 %s | FileCheck %s
Copy link
Copy Markdown
Contributor

@tex3d tex3d May 2, 2025

Choose a reason for hiding this comment

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

This now matches the first RUN line, so it seems to be pointless.
This test should probably be re-thought-through since the assumptions in the comments are now out of date.

Copy link
Copy Markdown
Collaborator Author

@bob80905 bob80905 May 8, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Understood, removed that Run Line. Cleaned up the comments too.
Fully intend to use the proposed dxil_dll_path option to augment these tests when I get around to that in a separate PR.

// This should always be run, and always succeed.
// CHECK: define void @main()

Expand Down
14 changes: 14 additions & 0 deletions tools/clang/test/Parser/deprecated-select-validator.hlsl
View file
Open in desktop
Copy link
Copy Markdown
Contributor

@tex3d tex3d May 8, 2025

Choose a reason for hiding this comment

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

I just realized that this is in the wrong location. This is a folder of clang c/cpp parser tests, not for testing the argument parsing. This whole folder is disabled for the DXC project, so this test will not run.

Check lit.local.cfg under Parser for:

config.suffixes = [] # HLSL Change - Disable lit suites.

This should be moved under the DXC folder instead.

Copy link
Copy Markdown
Collaborator Author

@bob80905 bob80905 May 8, 2025

Choose a reason for hiding this comment

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

Done!

Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
// Test that the deprecated option, select-validator, doesn't work.
// RUN: not %dxc -E main -T vs_6_7 -select-validator internal %s 2>&1 | FileCheck %s

// CHECK: dxc failed : Unknown argument: '-select-validator'

float4 main(int loc : SV_StartVertexLocation
, uint loc2 : SV_StartInstanceLocation
) : SV_Position
{
float4 r = 0;
r += loc;
r += loc2;
return r;
}
4 changes: 1 addition & 3 deletions tools/clang/test/SemaHLSL/hlsl/semantics/ExtendedCommandInformation/WrongShaderModel.hlsl
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
// TODO: use -verify instead of FileCheck after fix https://github.com/microsoft/DirectXShaderCompiler/issues/5768
// -select-validator internal used to avoid downlevel validator testing
// incompatibility with shader model 6.7.
// RUN: not %dxc -E main -T vs_6_7 -select-validator internal %s 2>&1 | FileCheck %s --check-prefix=SM67
// RUN: not %dxc -E main -T vs_6_7 %s 2>&1 | FileCheck %s --check-prefix=SM67

// SM67:invalid semantic 'SV_StartVertexLocation' for vs 6.7
// SM67:invalid semantic 'SV_StartInstanceLocation' for vs 6.7
Expand Down