Previous to this change if we modified arguments for C++ operator calls
we were sliding the modified parameters in the argument list relative to
where they should have been. This results in mismatched arguments in
calls and all sorts of nastiness.
* Clean up and rework DXIL library depencencies
This change reworks the dependency specifications for the DXIL*
libraries. Many of these libraries had over and under specified
dependencies.
This also collapses the DxilRDATBuidlder into the DxilContainer library
to remove the cyclic dependency between the two libraries.
* Break assert dependency between DXIL and Analysis
In assert builds the DXILModule constructor was creating forced bindings
to debugging methods that are often used from the debugger. This forced
binding is useful, but doesn't need to live in the DXIL library.
To break the dependency I've moved the code into Analysis. I've also
made that code only included when building with MSVC. When using other
compilers `__attribute__((used))` can be applied to the function via the
`LLVM_DUMP_METHOD` annotation to have the same effect.
This document tries to capture the basics of a unified build and test
workflow for all platforms. It is complete enough to start using but not
yet ready to be the default build mode.
This change fixes HLSL's copy-in/copy-out parameter passing. Before
this change we skip emitting copies for all local variable parameters,
which results in code correctness issues.
With this change, we do a poor-man's alias analysis walking each local
variable back to either an argument or an alloca. If it tracks back to
an argument marked noalias we can skip the copy. If it tracks to an
alloca, we skip the copy for the first parameter that tracks to that
alloca, and emit the copies for any subsequent parameters.
This change does have one side-effect that I don't understand. It
results in changing the ordering of some of the disassembly output. I
suspect that is caused by something in the disassembly being
order-dependent but haven't investigated fully.
Having this enabled makes dxc depends on ncurses5. This is required by
LLVM when supporting colored message output, but DXC doesn't supports it
for now.
Fixes#4892
ExecutionTests are currently compiled into clang-hlsl-tests.dll together with all of the other DXC unit tests. Later in the code pipeline the ExecutionTests are getting transformed into HLK tests which have different dependency constraints than the clang HLSL tests.
Refactoring ExecutionTests into a separate dll allows us to set its dependencies and build settings accordingly and closely matching those of the HLK test dll. This will help HLK authors to implement the tests with those constraints in mind.
PIX is unique in that it needs to deserialize, modify and then reserialize root sigs. The focus of this checkin is adding such modifications to PixPassHelpers.cpp. (This closes a gap in PIX support: PIX can now support shaders that use dxil-defined and attribute-style root signatures.)
But this required some work outside of the purely PIX-focused areas. Deserialized root sigs are described by a C-like structure with embedded arrays that are new/delete-ed by routines in DxilRootSignature.cpp. Since modifying these structs requires more new/delete calls, I chose to add a new entry point in DxilRootSignature.cpp to do the bare minimum that PIX needs: extend root params by one descriptor. This approach keeps all those raw new/deletes in one file.
I found a leak in DxilRootSignatureSerialzier.cpp, which I fixed. There appear to be no unit tests that exercise this path. I welcome feedback on adding one.
There were other leaks in class CVersionedRootSignatureDeserializer, but this class is unused so I deleted it.
Oh, and there are bazillions of commits because I was cherry-picking from a recent change (#4845) as it eveolved, since I needed that change and this to test PIX.
Custom half <-> float conversion functions had problems in multiple scenarios. This PR changes them into a wrapper, using the DirectXMath conversion functions instead.
Fix problem with expression under :failed subroutine, and preserve Failed variable on endlocal in :cleanup subroutine.
Use environment variable HLSL_TESTCMD_CLEANUP_ON_FAILURE to drive whether hcttestcmd cleans up temporary files on failure.
Defaults to 1 (true) unless it's already set.
You can set it to 0 to disable cleanup when the test fails.
Fix originally intended behavior for -Vi test and check files for cleanup.
This updates all the tests cases that rely on HLSL 2018 behaviors that
will break if the default HLSL version updates to 2021.
These tests are updated to be passed the HLSL 2018 version so they
continue testing and working under the same behaviors.
* Backport -ftime-trace from upstream llvm & clang
This change back-ports the -ftime-trace feature implemented by Aras
Pranckevičius (@aras-p) into DXC.
Aras has a great blog post describing the motivaiton for this feature
and how to use it (see:
https://aras-p.info/blog/2019/01/16/time-trace-timeline-flame-chart-prof
iler-for-Clang/).
As shaders are getting more and more complex we need better timing data
for ourselves as compiler writers and our users as HLSL writers to know
how to improve compile times of complex shaders.
This implementation is a starting point to give us data to help drive
our efforts to improve compile-time performance.
Because DXC's LLVM is very far removed from the version of LLVM the
patch was originally written for this change is not strictly a
cherry-pick of
https://github.com/llvm/llvm-project/commit/d880de2d19d46f7cfea7aa593602
458440bc1e8d. Instead this change is a manually applied and
reconstructed patch with significant alterations along the way. The
alaterations are performed by myself, but credit for the patch remains
with @aras-p.
Patch by Aras Pranckevičius (@aras-p).
This was found while moving to c++20 by an anonymous internal contributor.
Integrating the finding to mainline, and was already fixed upstream by @rnk (dd870f6929ee)
std::unique iterates on elements of the container, and removes all but
the first element matching the predicate, but does not resize the
container. This means the following code doesn't assert.
```cpp
std::vector<int> v = { 1, 1, 2, 2, 3, 3 };
assert(v.size() == 6);
std::unique(v.begin(), v.end());
assert(v.size() == 6);
```
The function returns a past-end iterator, and all the undefined elements
are at the end of the container, meaning we can safely erase them, and
fix the container size.
Signed-off-by: Nathan Gauër <brioche@google.com>
This patch attempts to translate HCTTest options to LIT build targets.
Since HCTTest and lit break up tests differently the mapping isn't quite
the same, but it should be close enough.
Before we can migrate our automation builds to using lit we need to
separate out the exec tests so that they can be disabled on the ADO
bots.
This modifies IDxcOptimizer::RunOptimizier to accept full DxilContainer input. When full container input is used, this restores some data that is stripped from the module and placed in various other container parts.
Data restored:
- Subobjects from RDAT
- RootSignature from RTS0
- ViewID and I/O dependency data from PSV0
- Resource names and types/annotations from STAT
Serialization of these to metadata in module bitcode output still requires hlsl-dxilemit step.
Double buffering a string stream writing in memory is silly. Upstream
LLVM made this change a few years ago based on analysis that most string
implementations are sane:
https://github.com/llvm/llvm-project/commit/65b1361
Previously, the test spirv.legal.sbuffer.counter.method.hlsl would cause errors if compiled with the DXIL backend:
```
../DirectXShaderCompiler/tools/clang/test/CodeGenSPIRV/spirv.legal.sbuffer.counter.method.hlsl:72:19: error: cannot map resource to handle.
float value = localBundle.getCSBuffer().Consume();
^
../DirectXShaderCompiler/tools/clang/test/CodeGenSPIRV/spirv.legal.sbuffer.counter.method.hlsl:87:5: error: cannot map resource to handle.
localWrapper.getASBuffer().Append(4.2);
^
../DirectXShaderCompiler/tools/clang/test/CodeGenSPIRV/spirv.legal.sbuffer.counter.method.hlsl:72:19: error: local resource not guaranteed to map to unique global resource.
float value = localBundle.getCSBuffer().Consume();
^
../DirectXShaderCompiler/tools/clang/test/CodeGenSPIRV/spirv.legal.sbuffer.counter.method.hlsl:87:5: error: local resource not guaranteed to map to unique global resource.
localWrapper.getASBuffer().Append(4.2);
^
../DirectXShaderCompiler/tools/clang/test/CodeGenSPIRV/spirv.legal.sbuffer.counter.method.hlsl:104:12: error: local resource not guaranteed to map to unique global resource.
return localRWSBuffer[5];
^
```
Fix these errors by assigning global resources to each local variable.
* [NFC] Move HLSL shfit behavior to frontend
The HLSL bit shift behavior was implemented in constant folding which
alters the semantics of the IR. Alternatively we can implement it in
the front end, leveraging the existing OpenCL code paths.
This results in the same final IR optimization and allows reverting the
ConstantFolder and IR tests back to match LLVM 3.7.
The HLSL shift behavior's optimized behavior is verified in existing
tests like the shift-fold.hlsl test, which is unimpacted by this change.
The big motiviation for this change is heading off subtle bugs. All our
tests cover cases where the shift's size is known at compile time. In
those cases the compiler can resolve the masking behavior
automatically. We have _no_ tests, nor compiler changes that impact
shifts that aren't compile-time known.
This means that prior to this change the behavior of shifting by a
compile-time resolvable value and shifting by a non-resolvable value
likely differs. Since backends often load DXIL and treat it as LLVM IR,
this could cause concerning issues.
* Address some low hanging fruit UBSan failures
Undefined Behavior Sanitizer causes hard failures on runtime conditions that exhibit undefined behavior as defined by C and C++. These issues may not cause immediately identifiable software errors, but the code behavior is dependent on the compiler and runtime implementation. As a result they may become difficult to diagnose or detect errors in the future if the compiler or runtime changes.
This change addresses occurrences of the following undefined behaviors.
* Performing arithmetic operations on null pointers.
* Calling memcpy with a source or destination pointer that is null.
* Calling memset with a destination pointer that is null.
* Assigning an enum a value which is not a valid enum for that enum.
* Reading from unaligned pointers.
After this change, there are 23 remaining undefined behavior failures in the current DXC tests which are less trivial to resolve.
This just surfaces and plumbs throug the -ftime-report clang flag in
dxc.exe.
-ftime-report prints timing data for different parts of the compile
to help identify and track down performance issues. When using the
dxcompiler library interface the output for the time report is returned
as a DXC_OUT_TIME_REPORT buffer.
I've set up the -ftime-report tests to run against the DXC command
line. On Windows this usees the hcttestcmds file, on Linux (and windows
if enabled) this will test through a lit shell suite.
High-level opcode is not stable, except within a build. Tests should not take a dependency on these opcode numbers.
Fixed the template/ByteAddressBufferLoad test that was failing due to this dependency.
While enums were introduced in HLSL 2017, this code should only be reached
on HLSL if we are HLSL 2021 or greater, so an additional language version
check does not appear necessary here.
A common source of bugs. Although many of these were harmless, some were
not. By eliminating them and enabling this warning as an error, we won't
add more in the future.
Fixed a real SPIRV bug that required a test update to expect the correct behavior.
SPIRV testing was expecting incorrect results that came from a fallthrough error
* Remove assumption that templates are never UDTs
There was an assumtion in the HLSL sema code that a template
specialization could never be a UDT. This assumption is incorrect now.
I've reworked the code so that we instead assume built-in types are
marked as `implicit` (which they all should and seem to be).
Correcting this in `IsHLSLNumericUserDefinedType` resulted in some
breakge in raytracing code generation because we used that method to
deterimine if structures could be payloads or attributes. That was an
incorrect API usage because we do have some builtin types that are
allowed.
The change here does the following:
* Introduce `IsHLSLBuiltinRayAttributeStruct` which returns true for the
builtin raytracing data types that behave like UDTs.
* Introduce `IsHLSLCopyableAnnotatableRecord` returns true for
user-defined trivially copyable structures and the builtin ray tracing
types.
* Adjust `IsHLSLNumericUserDefinedType` to do what the name says.
* Consolidates implementations of `IsUserDefinedRecordType` across
the project.
* Adds new test cases for the ray tracing built in structs to cover
diagnostic cases missed by the existing tests.
The new `IsHLSLBuiltinRayAttributeStruct` is hacky and uses the type
names (as the old code did). We should in the future insert an internal
attribute on the types that can be used to denote them so that we don't
need to match string names.
Resolves#4735
ID3D12FunctionReflection was leaving the `RequiredFeatureFlags` blank, but the information is available. This change fixes that omission.
Like the equivalent for ID3D12ShaderReflection (see `DxilShaderReflection::GetRequiresFlags`), the flags are the same between the `SHADER_FEATURE_*` flags and `D3D_SHADER_REQUIRES_*` flags, except that `D3D_SHADER_REQUIRES_EARLY_DEPTH_STENCIL` collides with `SHADER_FEATURE_COMPUTE_SHADERS_PLUS_RAW_AND_STRUCTURED_BUFFERS_VIA_SHADER_4_X` not exposed in `RequiredFeatureFlags`. So, the `D3D_SHADER_REQUIRES_EARLY_DEPTH_STENCIL` flag is patched in based on the flag captured in the shader props (`ShaderProps.PS.EarlyDepthStencil`) for library pixel shader entry points.
The `LLVM_REQUIRES_*` variables are internal variables in LLVM's
configuration used to determine if a build setting is required, they
don't actually control the build setting itself, so we shouldn't be
setting them.
`LLVM_ENABLE_*` variables control the build settings, and we do set
those correctly.
HLSL built-in templates have some special rules for verifying template
type parameters. These break with some common C++ template patterns.
This change applies HLSL template argument validation only to HLSL
implicit templates.
* [NFC] Transform all test cases to use `-HV 2021`
This just updates test cases to use `-HV 2021` instead of the
per-feature options.
* Remove HLSL 2021 per-feature flags
This change removes the HLSL 2021 per-feature enabling flags. Now that
HLSL 2021 is feature complete we're removing the ability to opt into
individual features. Any code relying on a single feature of HLSL 2021
should now adopt HLSL 2021 completely.
Co-authored-by: Greg Roth <grroth@microsoft.com>
This commit adds the logic in the SPIR-V backend to generate proper
bitfields. Bitfield are packed using a first-fit method, linearly
packing them, but not mixing types. Goal is to follow C/C++ rules.
Bitfield merging was initially guessed from the offset stored in the FieldInfo.
This offset is not always available and has a very specific meaning.
When the struct is a function local variable, the layout rule is Void,
meaning we shouldn't assume any kind of byte offset, but rely on
construct index.
This commit adds a fieldIndex member to the FieldInfo struct, and this
field is used to determine if 2 fields are merged.
When doing a buffer texture load, the struct must be extracted from a
vector type, and rebuilt. This commit adds support for bitfield extraction for such
types. Fixing this helped me see scalar assignment were also failling in
some cases. Addressing bitfield extraction/insertion issues on with
commit.
Signed-off-by: Nathan Gauër <brioche@google.com>
Co-authored-by: Cassandra Beckley <cbeckley@google.com>
Revert "spirv: get field index from SPIR-V type, not AST (#4806)"
This reverts commit 8f279baaef. This
commit isn't passing some of our internal tests and needs to be reverted
in preparation for the next Vulkan SDK release.
Exceptions for error handling can have a significant performance impact. The exception-throw codepaths are usually cold, and rely on large in-memory tables that need to be paged in.
This change to instead propagate returned error codes simplifies the code and allows the compiler to more aggressively optimize around both success and failure conditions.
This default method implementation is necessary for the base->derived size version mechanism to work.
This is only specialized when a base has a derived, and it relies on the newest version to use the default implementation (no specialization).
The implementation was removed in commit 8ee6ed19f3.
Note: existing path that limits operator to basic numeric types still allowed enum due to it being AR_TOBJ_BASIC, rather than using a new ArTypeObjectKind.
It's questionable whether this was intentional, but changing it would require another special case to allow enums, like exists for object types, and it might have other side effects as well.
Jeff Noyle
Xiang Li
Chris B
Nathan Gauër
Laura Hermanns
Noah Cabral
FantasqueX
Helena Kotas
Joshua Batista
Tex Riddell
Cassandra Beckley
Greg Roth
harshpg