-
Notifications
You must be signed in to change notification settings - Fork 711
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
Merge master to hlsl-2021 #3697
Conversation
ShaderOpTest::CreatePipelineState needs to check the return code from ID3D12Device2::CreatePipelineState so failures immediately stop here. Otherwise we might get a failure in a subsequent call like when CommandListRefs::CreateForDevice() calls ID3D12Device::CreateCommandQueue() - this one will fail with return code 0x887a0005 - The GPU device instance has been suspended. Use GetDeviceRemovedReason to determine the appropriate action.
An unrelated earlier change mistakenly removed the return on a line calling the Scatterer constructor for an argument value. As a result, the constructor is called, but the resulting object which would insert new extraction ops in the entry block is not used at all. Instead, the default case is encountered where the vector PHI op is used as an insertion point, which places the extractions after the new scalar PHIs that use them. This is caught in the loop case by an assert because the assumptions that caused LCSSA to be skipped for this BB are faulty.
Cycles in the dependency graph can result in phis and other instructions failing to get a wave sensitivity result after analysis. This can only happen when nothing in the loop has wave sensitive operations. By collecting phis that are unknown and marking them as non-sensitive after analyze is complete and rerunning it, we can correctly evaluate their dependants too. Because incompletely processed blocks might still be wave-sensitive, unknown phis are only added to be reprocessed when their preds have all be processed fully. In addition to cycles preventing wave sensitivity determination on dependant operations, they can prevent entering blocks that depend on values with cyclic dependencies. Even when reprocessing all unknown phis, this can result in not all values getting a determination. To resolve this, all blocks are added to the block list from the start to ensure that they are all processed eventually. Since instruction processing is more intelligent and targetted, this change is accompanied by a change that only processes a single block from the worklist before attempting to address queued instructions again. To catch wave-sensitive instructions sooner, when visiting an instruction, all arguments are checked for wave sensitivity even after an unknown argument is found whereas before an unknown argument ended the search. Add some tricky loop tests that failed in ToT and various intermediate stages of the change. Add a more aggressive assert to check for failures to find the wave-sensitivity. Fixed microsoft#2556 (again)
Comparing two unbound arrays where one was initilizing the other resulted in infinitely incrementing both and never reaching a conclusion. This adds detection to FlattenedTypeIterator for the situation whereupon the comparison is terminated and the result returned. A simple test of the assignment is added expecting the correct error.
microsoft#3549) merge from microsoft#3545 (cherry picked from commit 7e1a660) authored-by: Justin Holewinski <jholewinski@nvidia.com>
An esoteric build produced errors as a result of only using assigned variables in asserts. This makes a dummy use of them to quiet it.
The mesh shader path for QuadReadTest expects at least one render target. Without, the test framework attempts to clear a nonexistent RTV resource: D3D12 ERROR: ID3D12CommandList::ClearRenderTargetView: Specified CPU descriptor handle ptr=0xCCCCCCCCCCCCCCCC does not refer to a location in a descriptor heap. The commit in this PR simply adds an RTV, and will proceed through to test completion.
…ft#3558) * Take care case dynamic indexing on vector start with offset.
Instead of verifying that the args of an known phi were not known to be sensitive, it was checking the users, which can very well be wave sensitive
…mum precision variables (microsoft#3546) Currently, arrays of min16float etc. types don't get a RelaxedPrecision decoration for the OpVariable. This has some real performance implications since at least some hardware can fit more RelaxedPrecision variables into registers, and maybe also groupshared memory. This pull request add the missing RelaxedPrecision decoration for OpVariables of array types in a similar way vector and matrix minimum precision types are already handled.
Technically, the test matched the spec, but the D3D header didn't. I'm more interested in making this work than who was right though ;)
* Support -export-shaders-only for linker.
…nfo3 for DxcCompiler. (microsoft#3570)
This extension adds qualifiers for payload structures accompanied with semantic checks and code generation. This feature is opt-in for SM 6.6 libraries. The information added by the developer is stored in the DXIL type system and a new metadata node is emitted during code generation. The metadata is not necessary for correct translation of DXIL, so it may be safely ignored, but it provides hints to unlock potential optimizations in payload storage between DXR shader stages.
…icrosoft#3576) - VFS captures output files for duration of test, enabling: - %dxl test IDxcLinker - add -D to FileCheck args to supply defined variables - report failing RUN command when not consumed by FileCheck or XFail
…s = 0 (microsoft#3578) - Fix missing component type and count for Texture2DMS in ResProps - Fix reflection for Texture2DMS with default NumSamples
* Add -link to dxc.
…ing (microsoft#3586) * Use D3D12 headers from the same Windows 10 SDK version as cmake is using (cmake variable CMAKE_VS_WINDOWS_TARGET_PLATFORM_VERSION) * For ARM64EC builds detect SDK version from VSDevCmd environment variable instead of a fixed version in hctbuild.cmd * Add option -show-cmake-log to hctbuild and enable it in AppVeyor
dbg.value can occasionally return a null value. (Hit this in a customer (343) shader via PIX.) This is expected. From IntrinsicInst.cpp: // When the value goes to null, it gets replaced by an empty MDNode.
Allows running on limited platforms
microsoft#3656) spirv-val reports a validation error when we use variable offset for OpImage*. We have to rely on spirv-val for the validation instead of doing it in DXC. In particular, when the HLSL code uses the loop invariant variable for the Sample() intrinsics, it should be a constant value after conducting the loop unrolling. Since we rely on spirv-opt for the loop unrolling, we should not report the validation error in DXC. Fixes microsoft#3575
* Save root sig for entry for lib.
…rosoft#3664) Change the mapping of SV_ShadingRate from FragSizeEXT (VK_EXT_fragment_density_map) to PrimitiveShadingRateKHR/ShadingRateKHR (VK_KHR_variable_rate_fragment_shading).
) When calling a method using an object, we have to correctly pass "this" object. When the class of the object used for the method call has multiple base classes, it does not use the correct base class. It simply uses "this" object. This commit fixes the issue. Fixes microsoft#3644
…serts/crashes (microsoft#3683) When a diagnostic that should be emitted with DiagnoceOnce has already been emitted, an invalid DiagnosticsBuilder is returned to prohibit another emittance of the diagnostic. The current implementation, however, does not set IsActive=false and the DiagnosticsEngine tries to emit a diagnostic with an invalid DiagID. This function is currently only used with payload access qualifiers to emit a warning only once if qualifiers are dropped because PAQs are disabled because of the SM version or the user opted-out. (Update the corresponding test accordingly).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I have definitely reviewed every line of this change and confirmed that they match the changes submitted to the main branch character for character 😅
❌ Build DirectXShaderCompiler 1.0.19 failed (commit b6b0d537e3 by @hekota) |
…rCompiler into merge-master-hlsl-2021
f91169a
to
af08851
Compare
❌ Build DirectXShaderCompiler 1.0.25 failed (commit dd2b00f474 by @hekota) |
Author: Adam Yang
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the changes are fine as-is, but I've suggested a slightly better organization.
case BuiltinType::LitFloat: | ||
llvm_unreachable("Unsupported HLSL types"); | ||
Encoding = llvm::dwarf::DW_ATE_float; | ||
break; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The fault really lies with me for adding these here in the first place, but a better place for these would be the HLSL-specific sections of cases below including Min16Int and HalfFloat respectively. This #else
block is meant for ObjC and OCL stuff.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
@@ -1,5 +1,6 @@ | |||
// RUN: %dxc -E main -T ps_6_0 -enable-templates %s | FileCheck %s | |||
// RUN: %dxc -E main -T ps_6_0 %s -enable-templates -DCHECK_DIAGNOSTICS | FileCheck %s -check-prefix=DIAG | |||
// RUN: %dxc -E main -T ps_6_0 -enable-templates %s /Zi |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No Filecheck, intended just to make sure it doesn't crash?
✅ Build DirectXShaderCompiler 1.0.52 completed (commit 0c3ca67ec2 by @hekota) |
If you look at the merge commit, you can see the conflict resolution (changes introduced during merge in addition to automatic merge of incoming changes), though I'm not sure how you can do this in GitHub. Here they are:
Just straight-forward adjacency conflict resolutions, it would appear. The rest of the differences are the already-verified incoming commits, and the additional commits in this PR. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
✅ Build DirectXShaderCompiler 1.0.53 completed (commit 3b3465a546 by @hekota) |
Fixes #3696