forked from microsoft/DirectXShaderCompiler
- Notifications
You must be signed in to change notification settings - Fork 0
Update dxc with Nabla changes to latest as of 03/01/24 #2
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
Merged
devshgraphicsprogramming merged 203 commits into devshFixes from update_nbl_dxc_03_01_24 Jan 12, 2024
Merged
Update dxc with Nabla changes to latest as of 03/01/24 #2
devshgraphicsprogramming merged 203 commits into devshFixes from update_nbl_dxc_03_01_24 Jan 12, 2024
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Add Shader Model 6.8 features in preview state: Work Graphs and Wave Matrix. This code is considered preview ready, with more work planned before retail shaders can be compiled to Shader Model 6.8.
…version test Replace incorrect sizeof() with strlen() in compiler version test This would produce a failure only on x86 because it sizeof() gave the size of a pointer, which just so happens to be the hard coded size of the hash on x64. On x86, it would advance only 4 bytes, and fail the comparison. This also checks to see if the VersionStringListSizeInBytes is > 2 more than the hash size, because both null-terminators are always added, so it will always be at least 2 more, even with no CustomVersionString.
The versioning is broken if the extra space in there.
Test relies on validator version 1.8, but doesn't specify this version requirement. Added requirement.
GCC failed when faced with a variable of both type and name RecordDispatchGrid. Adding struct to the type to make it clear eliminates the error
…ctionProps (microsoft#5335) A recent change from a recent PR includes changes to how the MDVals data structure inside of EmitDxilFunctionProps gets elements (push_back instead of index assignment). However, there was no accompanying test for these changes, because HLSLFileCheck\hlsl\workgraph\called_function_arg_nodeoutput.hlsl exercised this code path and failed without the MDVals changes. This PR adds some extra context to the above test so that the extra purpose the above test serves isn't lost with time. The test passes with the MDVals pushback changes, and fails with the original valIdx index assignments.
Globals.h defines things like `IFC` macros that leak into callers. All that was really needed here was a definition of `std::string`.
This will avoid test fail when forget to add git usr bin to PATH. Port from llvm/llvm-project@0f1f13f
This is for case where git usr bin is not in winreg.
…ccepting malformed handle arguments (microsoft#5399) Except for Output Complete, instructions should not accept handle arguments that are undefs / zeroinitializers, and should emit a diagnostic when this happens. This PR adds tests to specifically exercise the cases where a call instruction : 1. receives an undef argument 2. receives a zeroinitializer argument 3. receives a handle argument that is invalid And combines these cases with all different types of handle arguments (resource handle, node handle, and node record handle) This PR intends to at least implement the first work item in [microsoft#5356](microsoft#5356)
…cy (microsoft#5336) This test used the ShaderOpArithTable.xml in a weird way that breaks the way we map to the HLK test. Use of ShaderOpArithTable.xml in this test was unnecessary, since it wasn't using more than one row with different parameter sets to define data-driven test cases. This change simplifies things and gets rid of this dependency. The impact is that the shaders are now defined in ShaderOpArith.xml in place of the dummy shaders that used to be there, instead of ShaderOpArithTable.xml. The shader text and target are used as defined from ShaderOpArith.xml, instead of overriding those values with ones from ShaderOpArithTable.xml. The only changes to the shader content during the move is in whitespace: indentation changed to be consistent with target file, trailing whitespace removed.
This will avoid test fail when forget to add git usr bin to PATH. If git usr bin already on PATH, it will just use it. Port from llvm/llvm-project@0f1f13f
Issue 5372 highlighted obsolete code left over from an earlier version of work graph implementation. After investigation it is confirmed that none of the changes there are currently required, so this change reverts to the previous version of the function, and also updates the related test. Fixes microsoft#5372 Co-authored-by: Tim Corringham <tcorring@amd.com>
Diagnostics for invalid shader attributes to shader functions were appearing too late. This PR moves the emission of these diagnostics earlier so the verifier can catch them, and adds some tests to accompany the changes. This PR is one of a few that will help implement this task: microsoft#5368
…osoft#5484) This PR is one of several more PR's that move diagnostic emission earlier for specific errors, so that the verifier can catch these errors. This PR focuses on the MaxRecord* attributes applied on parameters. Specifically, there are 2 behaviors that this PR will allow DXC to detect earlier: 1. Prevent more than one of {MaxRecord, MaxRecordsSharedWith} from being applied to a single parameter 2. Prevent MaxRecordsSharedWith from referencing the same parameter name that the attribute is being applied to. Some tests have been merged or deleted to consolidate these verifier tests. There is still some behavior that cannot be tested at this time (verifying that MaxRecordsSharedWith doesn't reference an invalid parameter name), which will be addressed in the future. It's likely that this location is too early to properly validate this behavior, and the tests for this behavior may need to be left at the same location. This PR is one of a few that will help implement this task: microsoft#5368
Some D3DReflect tests compare size values literally, when the size will change depending on the validator version. This PR changes the literal integer in the checkline to a regex, so that the size value changing between validators won't break the test. Fixes microsoft#5386
The wave matrix tests had a remaining __debug_break() call left in the code. There was also a function that risked returning the wrong comparison value due to underflow behavior. This PR improves the stability of the wave matrix tests by removing the __debug_break() call and preventing underflow from happening. Fixes microsoft#5498 Fixes microsoft#5369
Update both the verifier and validator tests for the node and compute compatibility errors. The front-end error diagnostics and the validator checks for node and compute incompatibilities are already present in the preview branch. This updates both the verifier and validator test cases to make them more comprehensive. Fixes microsoft#5346 --------- Co-authored-by: Tim Corringham <tcorring@amd.com>
…ft#5577) Move work graph related diagnostics from CodeGen to Sema in order to provide earlier detection of invalid code, which will help ensure that the generated AST is valid. Also replace tests for these with verifier tests. Fixes microsoft#5352 --------- Co-authored-by: Tim Corringham <tcorring@amd.com> Co-authored-by: Greg Roth <grroth@microsoft.com> (cherry picked from commit 327e582) Co-authored-by: Tim Corringham <timothy.corringham@amd.com>
IsHLSLNodeIOType is supposed to capture all node object types, but was missing output record and output array types. TranslateHLCastHandleToRes had LLVM_FALLTHROUGH in for two cases where it made no sense to fall through to the next case. MergeGepUse is needed after lowering GetRecordPtr to collapse GEP chains that could result in intermediate multi-dimension array pointers from GEPs, which is considered an invalid DXIL type. Updated some tests for -Od compilations. Added a test for the multi-dim GEP case. Some CHECKs needed specialization for -Od: - align 4 added to load/store only with optimizations enabled - no CSE of index/annotate node handle - also, use CHECK-LABEL to make failure investigations easier Fixes microsoft#5597
The entry diagnostics in SemaDXR in SM 6.8 had been expanded to include diagnostics for entry functions that are not related to ray tracing - particulary node diagnostics. This change relocates the diagnostics unrelated to ray tracing to SemaHLSL. Fixes microsoft#5528 Co-authored-by: Tim Corringham <tcorring@amd.com>
The team has come to a decision that no inferences should be made on function declarations that have a numthreads attribute (specifically, inferences on whether or not the function represents a shader, or a compute shader, even). However, some tests exist that were written with the expectation that they would be treated as a compute shader, and they lack the compute shader attribute. This PR addresses the problem by correctly adding the compute shader attribute to the function declarations in these tests. The PR is necessary so that these tests won't fail when the inference behavior is removed. Fixes microsoft#5623
EmitInstrNote reused most of the code from EmitInstrErrorMsg. This consolidates both functions into one that shares most of the code. In addition, it no longer makes the note look like an additional error in the output and adds testing for that note. Relevant tests are modified to include the notes Fixes microsoft#5613
Conflicts primarily from differences in how the compiler version mismatch on linking was implemented, some additional basic types added in one place and the other, and some incidental clang-formatting along the way in main
…tion (microsoft#5679) There was a recent spec change on work-graphs that disallows the existence of node + compute attributes on shader declarations. Because of this old behavior, a lot of code worked around the possibility that 2 shader attributes could exist for one declaration. With this old behavior disallowed, we no longer need to account for this possibility. The PR now adds a diagnostic preventing multiple attributes from existing. It will also change all the tests that worked with shaders with multiple shader attributes, and either remove them, or update them so that only one attribute is used. Fixes microsoft#5601
Elimination of incorrect output complete calls was located in an awkward spot. The code has been moved over to DCE. Fixes microsoft#5363
…on (microsoft#5627) This PR will automatically add the target profile shader stage as a HLSLShader attribute to the function declaration that represents the entry point. If the entry point doesn't exist (in the case of library shaders), then this behavior won't happen. Fixes microsoft#5626
microsoft#5679 removed some code in the CodeGen pass that allowed node + compute, and some tests were updated. In this follow up PR, code is changed in the Sema pass that used to allow validation exemptions for node + compute. The appropriate tests were updated. This is an improvement to microsoft#5601 Fixes microsoft#5601
…soft#6069) Convert the moved shaders to lit FileCheck test. 5 shaders not used by runFileTest are also converted and left in CodeGenSPIRV with CodeGenSPIRV folder enabled for lit. Now all shaders are converted to lit. Will move all files back to CodeGenSPIRV and delete CodeGenSPIRV_Lit next.
Move shaders out of CodeGenSPIRV_Lit and delete CodeGenSPIRV_Lit.
…crosoft#5995) Not all matrix representations could be generated with current dxc, so use IR test here. Fixes microsoft#5696
FindD3D12.cmake sets WIN10_SDK_VERSION to CMAKE_VS_WINDOWS_TARGET_PLATFORM_VERSION; however, this latter variable is only defined when using a Visual Studio CMake generator. When using Ninja instead, for example, this variable will be empty, and WIN10_SDK_VERSION will remain unset. This change detects when CMAKE_VS_WINDOWS_TARGET_PLATFORM_VERSION and attempts to retrieve the latest SDK by listing directories under ${WIN10_SDK_PATH}/Include. Make the table with supported shader model, major and minor numbers visible.
While emitting diagnostic notes about conversions, the code for checking `OutConversions` was not included in the loop. This caused an OOB when `I >= NumConversions`. Normally, this does not affect anything (other than not emitting the diagnostics for `OutConversions`), since most of the time `OutConversions[I].isBad()` happens to return false. However, in some cases when `OutConversions[I].isBad()` is true, calling `DiagnoseBadConversion()` on the invalid entry will crash.
…icrosoft#6074) This PR improves the experience for profilers and debuggers that consume SPIR-V produced by dxc. When debugging information is output, additionally note the beginning of the internally generated wrapper function. This will assign all parameter setup to the entry point declaration, providing context to the user of a shader profiler or debugger.
The `createStageVars` function is very large, and difficult to fix. In support of two bug fixes, I want to refactor the function so try to make the code cleaner. This change will try to keep the exact same functionality. I believe their could be further simplification after this if needed.
The CodeGenSPIRV_Lit directory was renamed, but the appveyor script was not updated. This PR updates it.
…t#6058) It is unclear what should happen if an input parameter on a function is marked as precise. I am choosing to propagate it to the actual parameters. Note that it is not wrong to propagate precise more than we should since it will only stop some optimizations.
The ExtractRecordStructFromArray intrinsic is left over dead code that should be removed. This PR removes the intrinsic from existence. In the cases where this intrinsic was used, DefaultSubscript is used instead. Moved from microsoft#5893 to target the main branch instead. Fixes microsoft#5351
Update table with supported shader model name, major and minor version number. Replace "verifier" with "validator"
The IsRay() function defined in the ShaderModel class can be misleading. It is included in a set of functions that are used to test whether or not the shader in question was targeted to a specific shader stage. However, it is impossible for a shader to be targeted to, say, the "raygeneration" stage, or any other stages associated with the kinds that the IsRay() function checks for. Like the "node" shader kind, a shader can only have these shader kinds if the associated attribute is found on the function declaration. To prevent confusion, and to keep the Is*() functions inside the ShaderModel class restricted to only those targetable shader stages, the IsRay() function can be removed. This PR comes as a response to microsoft#6008
…uint64_t to prevent overflow (microsoft#6090) Previously, in order to determine whether the three arguments provided to the MaxDispatchGrid attribute exceeded the maximum product value (which is 16,777,215 (2^24-1)), each of the three components would be multiplied together and compared to this maximum. However, the three arguments would be read in as unsigned ints. In certain conditions, the multiplication would cause an overflow, which would programmatically result in a number less than the maximum product limit, despite mathematically being above this limit. The diagnostic should trigger in this case, but it doesn't. This PR changes the data type of the three arguments to unsigned long long, which can hold the maximum possible product that can be achieved without triggering the per-multiplicand limit of 65535, and won't overflow in this case. Now, the compiler will trigger this diagnostic. Regression test has been added. Fixes microsoft#5988
…rosoft#6089) WaveMatrix wasn't checking WaveMMATier before running tests (assuming it was always supported for SM 6.8). This change fixes this. D3D12_SDK_VERSION check for WAVE_MMA feature structure and enum definitions assumed they will be defined in SDK version 613. This isn't accurate, so this block of local definitions will always be enabled until we have the correct version in the future.
In DxcPixLiveVariables.cpp, UniqueScopeForInlinedFunctions::AscendScopeHierarchy tries to keep track of a variable's scope and the scope at which its containing function was inlined, if any. This scope duo has to be the same in two places for things to work out: the scopes inferred from dbg.declare, and then again when the scope hierarchy is ascended in order to discover all variables accessible from a given point in the program. AscendScopeHierarchy was forgetting to update the inlined part of the scope, resulting in, for example, PIX's debugger losing track of variables outside of, for example, a for loop, even though the code can see those variables at that point. Just setting the inlined scope to be the same as the function scope fixes this problem. (In addition to these new tests, existing PIX tests exercise this situation happily)
…icrosoft#6095) This reverts commit 36bfa74. CodeGenSpirvTest.cpp has been cut down significantly from its original >10,000 line size, so this flag should no longer be necessary. Fixes microsoft#3867
The next step in the refactoring is to reduce the number of variables passed to various functions. This should make the logic easier to follow. This is done by grouping variables into a struct, and we pass the struct along. --------- Co-authored-by: Cassandra Beckley <cbeckley@google.com>
…ee (microsoft#6077) Convert runCodeTest to lit FileCheck test and remove all the code related to runCodeTest. Remove effcee and re2 from git submodule since nothing is dependent on them after the change.
Remove unused function VerifyOutputWithExpectedValueUInt4.
This codebase is using C++17, but to remain compatible with C++20, I need to remove this. Starting C++20, an aggregate is required to have no user-declared constructors, vs before, no user-provided constructors. So even if if the only the default constructor is provided, this is enough to prevent us from using this class as an aggregate in c++20. Signed-off-by: Nathan Gauër <brioche@google.com>
…icrosoft#6027) I definitely think it would look better if we allowed these attributes on variables, ie microsoft/hlsl-specs#76. I haven't fully investigated how involved it would be to implement, but my intuition is that it wouldn't take that much more work. Fixes microsoft#4217.
Bump the SPIRV-Tools version to v2023.6.rc1 to prepare Vulkan SDK release.
…iler into update_nbl_dxc_03_01_24
| PR description is empty, please add some valid description |
You can test this locally with the following command:git-clang-format --diff 024c8a9a349dc45f5b4818c413502e0a45f5d542 e33460f3bf9b0863b3b5cd09804b4048fa03bb73 -- include/dxc/DXIL/DxilNodeProps.h include/dxc/DXIL/DxilWaveMatrix.h lib/DXIL/DxilNodeProps.cpp lib/DXIL/DxilWaveMatrix.cpp tools/clang/lib/SPIRV/PervertexInputVisitor.cpp tools/clang/lib/SPIRV/PervertexInputVisitor.h tools/clang/unittests/HLSL/PixDiaTest.cpp tools/clang/unittests/HLSL/PixTestUtils.cpp tools/clang/unittests/HLSL/PixTestUtils.h include/dxc/DXIL/DXIL.h include/dxc/DXIL/DxilConstants.h include/dxc/DXIL/DxilFunctionProps.h include/dxc/DXIL/DxilInstructions.h include/dxc/DXIL/DxilMetadataHelper.h include/dxc/DXIL/DxilOperations.h include/dxc/DXIL/DxilSemantic.h include/dxc/DXIL/DxilShaderFlags.h include/dxc/DXIL/DxilShaderModel.h include/dxc/DXIL/DxilTypeSystem.h include/dxc/DXIL/DxilUtil.h include/dxc/DxilContainer/DxilPipelineStateValidation.h include/dxc/DxilContainer/DxilRuntimeReflection.h include/dxc/HLSL/HLModule.h include/dxc/HLSL/HLOperationLower.h include/dxc/HLSL/HLOperations.h include/dxc/HlslIntrinsicOp.h include/dxc/Support/ErrorCodes.h include/dxc/Support/HLSLOptions.h include/dxc/Test/DumpContext.h include/dxc/Test/DxcTestUtils.h include/dxc/Test/HlslTestUtils.h include/dxc/WinAdapter.h include/dxc/dxcapi.internal.h include/dxc/dxcpix.h include/llvm/InitializePasses.h include/llvm/Transforms/Scalar.h lib/DXIL/DxilMetadataHelper.cpp lib/DXIL/DxilModule.cpp lib/DXIL/DxilOperations.cpp lib/DXIL/DxilResourceProperties.cpp lib/DXIL/DxilSemantic.cpp lib/DXIL/DxilShaderFlags.cpp lib/DXIL/DxilShaderModel.cpp lib/DXIL/DxilTypeSystem.cpp lib/DXIL/DxilUtil.cpp lib/DxcSupport/HLSLOptions.cpp lib/DxcSupport/WinAdapter.cpp lib/DxilContainer/DxilContainerAssembler.cpp lib/DxilContainer/RDATDumper.cpp lib/DxilDia/DxcPixLiveVariables.cpp lib/DxilDia/DxilDiaSession.cpp lib/DxilPIXPasses/DxilDebugInstrumentation.cpp lib/HLSL/DxilContainerReflection.cpp lib/HLSL/DxilGenerationPass.cpp lib/HLSL/DxilValidation.cpp lib/HLSL/HLLowerUDT.cpp lib/HLSL/HLModule.cpp lib/HLSL/HLOperationLower.cpp lib/HLSL/HLOperations.cpp lib/HLSL/HLSignatureLower.cpp lib/HLSL/HLSignatureLower.h lib/Transforms/IPO/PassManagerBuilder.cpp lib/Transforms/Scalar/LowerTypePasses.cpp lib/Transforms/Scalar/ScalarReplAggregatesHLSL.cpp lib/Transforms/Utils/Local.cpp tools/clang/include/clang/AST/HlslTypes.h tools/clang/include/clang/Basic/DiagnosticIDs.h tools/clang/include/clang/Basic/LangOptions.h tools/clang/include/clang/SPIRV/FeatureManager.h tools/clang/include/clang/SPIRV/SpirvBuilder.h tools/clang/include/clang/SPIRV/SpirvFunction.h tools/clang/include/clang/SPIRV/SpirvInstruction.h tools/clang/include/clang/SPIRV/SpirvModule.h tools/clang/include/clang/Sema/Sema.h tools/clang/include/clang/Sema/SemaHLSL.h tools/clang/lib/AST/ASTContextHLSL.cpp tools/clang/lib/AST/HlslTypes.cpp tools/clang/lib/Basic/DiagnosticIDs.cpp tools/clang/lib/CodeGen/CGDebugInfo.cpp tools/clang/lib/CodeGen/CGExpr.cpp tools/clang/lib/CodeGen/CGHLSLMS.cpp tools/clang/lib/CodeGen/CGHLSLMSFinishCodeGen.cpp tools/clang/lib/CodeGen/CGHLSLMSHelper.h tools/clang/lib/Frontend/ASTUnit.cpp tools/clang/lib/Parse/ParseDecl.cpp tools/clang/lib/SPIRV/CapabilityVisitor.cpp tools/clang/lib/SPIRV/DeclResultIdMapper.cpp tools/clang/lib/SPIRV/DeclResultIdMapper.h tools/clang/lib/SPIRV/EmitVisitor.cpp tools/clang/lib/SPIRV/EmitVisitor.h tools/clang/lib/SPIRV/FeatureManager.cpp tools/clang/lib/SPIRV/InitListHandler.cpp tools/clang/lib/SPIRV/PreciseVisitor.cpp tools/clang/lib/SPIRV/PreciseVisitor.h tools/clang/lib/SPIRV/SpirvBuilder.cpp tools/clang/lib/SPIRV/SpirvEmitter.cpp tools/clang/lib/SPIRV/SpirvEmitter.h tools/clang/lib/SPIRV/SpirvInstruction.cpp tools/clang/lib/SPIRV/SpirvModule.cpp tools/clang/lib/Sema/SemaDXR.cpp tools/clang/lib/Sema/SemaDecl.cpp tools/clang/lib/Sema/SemaHLSL.cpp tools/clang/lib/Sema/SemaOverload.cpp tools/clang/tools/dxcompiler/dxcdisassembler.cpp tools/clang/tools/dxcompiler/dxcompilerobj.cpp tools/clang/tools/dxcompiler/dxcutil.cpp tools/clang/tools/dxcompiler/dxcutil.h tools/clang/tools/libclang/dxcrewriteunused.cpp tools/clang/unittests/HLSL/CompilerTest.cpp tools/clang/unittests/HLSL/DxilContainerTest.cpp tools/clang/unittests/HLSL/PixTest.cpp tools/clang/unittests/HLSL/RewriterTest.cpp tools/clang/unittests/HLSL/ValidationTest.cpp tools/clang/unittests/HLSL/VerifierTest.cpp tools/clang/unittests/HLSLExec/ExecutionTest.cpp tools/clang/unittests/HLSLExec/ShaderOpTest.cpp tools/clang/unittests/HLSLExec/ShaderOpTest.hView the diff from clang-format here.diff --git a/tools/clang/lib/SPIRV/EmitVisitor.cpp b/tools/clang/lib/SPIRV/EmitVisitor.cpp index 17b3c1ed0..33df2c00c 100644 --- a/tools/clang/lib/SPIRV/EmitVisitor.cpp +++ b/tools/clang/lib/SPIRV/EmitVisitor.cpp @@ -263,7 +263,7 @@ void EmitVisitor::emitDebugLine(spv::Op op, const SourceLocation &loc, // created by DXC. We do not want to emit line information for their // instructions. To prevent spirv-opt from removing all debug info, we emit // OpLines to specify the beginning and end of the function. - if (inEntryFunctionWrapper && + if (inEntryFunctionWrapper && (op != spv::Op::OpReturn && op != spv::Op::OpFunction)) return; |
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Add this suggestion to a batch that can be applied as a single commit. This suggestion is invalid because no changes were made to the code. Suggestions cannot be applied while the pull request is closed. Suggestions cannot be applied while viewing a subset of changes. Only one suggestion per line can be applied in a batch. Add this suggestion to a batch that can be applied as a single commit. Applying suggestions on deleted lines is not supported. You must change the existing code in this line in order to create a valid suggestion. Outdated suggestions cannot be applied. This suggestion has been applied or marked resolved. Suggestions cannot be applied from pending reviews. Suggestions cannot be applied on multi-line comments. Suggestions cannot be applied while the pull request is queued to merge. Suggestion cannot be applied right now. Please check back later.
No description provided.