-
Notifications
You must be signed in to change notification settings - Fork 463
Default slang-test compiler invocations to -O0 #11805
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
base: master
Are you sure you want to change the base?
Changes from 3 commits
14b7ba9
968453a
ef2ec5f
eab84de
ff5fc34
5b4c9df
d3a525f
265ad27
1f05451
3e6ebdf
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5,5 +5,5 @@ | |
| //DIAGNOSTIC_TEST:SIMPLE(diag=CHECK1):-target -profile ps_4_0 | ||
| //CHECK1: unknown code generation target '-profile' | ||
|
|
||
| //DIAGNOSTIC_TEST:SIMPLE(diag=CHECK2):-profile ps_4_0 -target | ||
| //DIAGNOSTIC_TEST:SIMPLE(diag=CHECK2):-profile ps_4_0 -O3 -target | ||
|
jkwak-work marked this conversation as resolved.
Outdated
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This doesn't make sense.
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Agent] Agreed — fixed at the root in 265ad27 and this test file is now unchanged from master. The real problem was that slang-test appended its default -O0 after the directive arguments, so a diagnostic test ending with an intentionally dangling -target would consume -O0 as its argument and report a different diagnostic. addDefaultSlangOptimization now inserts the default at the front of the argument list (slangc option parsing is position-independent), so test-provided arguments keep their meaning and no test needs to compensate. Validation: the slangTestDefaultOptimizationInsertion unit test now pins the front-insertion behavior with a dangling -target case, and tests/diagnostics/, tests/front-end/, and tests/cross-compile/ all pass locally (756/756). |
||
| //CHECK2: expected an argument for command-line option '-target' | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,7 +2,7 @@ | |
| // marshalling: the outer array elements are themselves arrays, each of | ||
| // which must be packed element-by-element into AnyValue storage. | ||
|
|
||
| //TEST(compute):COMPARE_COMPUTE_EX(filecheck-buffer=CHECK):-dx12 -compute -shaderobj -output-using-type | ||
| //TEST(compute):COMPARE_COMPUTE_EX(filecheck-buffer=CHECK):-dx12 -compute -shaderobj -output-using-type -Xslang -O2 | ||
|
jkwak-work marked this conversation as resolved.
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This should be
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Agent] I tried -Xslang -O3 and it fails locally on dx12: dxc validation rejects the shader with 'Instructions should not read uninitialized value' at 'select i1 %15, float fadd (float fadd (float undef, float undef), float undef), ...' in computeMain, so the compute run returns no output and the CHECK comparison fails. At -O2 the test passes (5/5 locally, and it passed CI). So -O2 and -O3 do differ on the DXIL path: the Slang level maps to different downstream dxc optimization flags, and dxc at the higher level exposes an undef read in the generated HLSL. I kept -O2 for now. Two follow-ups for your call: (1) confirm keeping -O2 here, and (2) the undef read that dxc surfaces at -O3 may be a real codegen issue in the dynamic-dispatch/AnyValue path worth filing separately.
jkwak-work marked this conversation as resolved.
|
||
| //TEST(compute):COMPARE_COMPUTE_EX(filecheck-buffer=CHECK):-vk -compute -shaderobj -output-using-type | ||
| //TEST(compute):COMPARE_COMPUTE_EX(filecheck-buffer=CHECK):-cuda -compute -shaderobj -output-using-type | ||
| //TEST(compute):COMPARE_COMPUTE_EX(filecheck-buffer=CHECK):-cpu -compute -shaderobj -output-using-type | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.