This commit adds support for Sampler/Resource descriptor heaps in DXC.
Support for those heaps on the SPIR-V side requires no other extension
than SPV_EXT_descriptor_indexing.
On the Vulkan side, the VK_EXT_mutable_descriptor_type will be required
as multiple descriptor types must be allowed on the same binding.
When loading a type from a heap, DXC generates a new OpRuntimeArray of
the correct type, and binds it to `set=0,
binding=<BindingNumberOfTheHeap>`.
This means multiple OpRuntimeArrays will share the same binding. This is
why VK_EXT_mutable_descriptor_type is required.
This implementation uses at most 3 bindings:
- N OpRuntimeArray as binding A for the ResourceDescriptorHeap
- N OpRuntimeArray as binding B for the SamplerDescriptorHeap
- 1 OpRuntimeArray %counter_type for the ResourceDescriptorHeap
counters.
The bindings are only allocated if used. If only the
SamplerDescriptorHeap is used, a single binding is required.
The binding allocation logic is:
1. allocate bindings for every resources, excluding heaps.
2. If ResourceDescriptorHeap is used, find the first unused binding in
set=0 and use it.
3. Same for the SamplerDescriptorHeap
4. Same for the counters.
UAV counters are not always created, only if used.
When used, they are stored in an OpRuntimeArray. The index of a counter
in that array
is equal to the index of the associated resource in its own
OpRuntimeArray.
```hlsl
RWStructuredBuffer a = ResourceDescriptorHeap[2];
a.IncrementCounter();
// buffer in descriptorSet 0, binding 0, OpRuntimeArray[index=2]
// counter in descriptorSet 0, binding 1, OpRuntimeArray[index=2]
```
As-is, this PR doesn't allow resource heaps to alias regular resources,
or to overlap.
A follow-up PR will add 3 flags to override each binding/set pairs:
- 'fvk-bind-resource-heap <set> <binding>'
- 'fvk-bind-sampler-heap <set> <binding>'
- 'fvk-bind-counter-heap <set> <binding>'
---------
Signed-off-by: Nathan Gauër <brioche@google.com>
When the prototype of an entrypoint was defined, the codegen crashed
because it failed to filter this partial declaration when adding
functions to the work-queue.
Fixes#6750
Signed-off-by: Nathan Gauër <brioche@google.com>
We should use the pull_request_target option here so that the PR runs
from the pipeline in the target rather than the PR source branch. This
allows the action to run with reduced security implications.
This fixes a bug where the offsets for elements in vectors with 16-bit
types doesn't take into account alignment bits and PIX wouldn't display
vector element values correctly in the shader debugger. Eg. if
`-enable-16bit-types` wasn't set, the offsets for a min16float4 would be
0, 16, 32, 48 instead of 0, 32, 64, 96.
Also removed the assert in PopulateAllocaMap_StructType that was
checking whether the calculated aligned offset matches the packed offset
(from SortedMembers) because it was false for members with sizes smaller
than the alignment size.
In CalcResTypeSize function, UNREFERENCED_PARAMETER macro is used with a
parameter which is referenced in return statement. That macro was added
in
6ee4074a4b but it was not removed when
"DxilModule &M" was referenced in
86073a3b0b
This fixes the following compiler error with clang 18.1.8 with mingw-w64
toolchain.
DxilContainerReflection.cpp:1512:3: error: object of type 'DxilModule'
cannot be assigned because its copy assignment operator is implicitly
deleted
1512 | UNREFERENCED_PARAMETER(M);
| ^
winnt.h:1387:40: note: expanded from macro 'UNREFERENCED_PARAMETER'
1387 | #define UNREFERENCED_PARAMETER(P) {(P) = (P);}
| ^
The code that implements `RWByteAddressBuffer::Store` will iterate over
all of the fields in a struct to write each element in the struct.
However, it does not use the "Spir-V fields", which accounts for
multiple fields being packed into the same bitfield. This is fixed by
using the `forEachSpirvField` function to make sure that the bitfield
are correctly handled.
Fixes#6483
When a scalar variable is passed as the argument to an inout vector
parameter,
then the scalar is suppose to be splatted. After returning from the
function, we need to extract the first element from the parameter to
store back into the scalar.
Fixes#6568
When the [branch] annotation is used, switches are converted into an
if/else tree.
Issue arise when declaring a variable into a switch case:
- when flattened, the same variable could be traversed twice in case of
a case fall-through.
In addition, there was a small bug in the flattening logic: only breaks
were stopping the handling of further statements, while early-returns
were ignored. This was not an important bug as it only added more
dead-code, but it was wrong.
Fixes#6718
Signed-off-by: Nathan Gauër <brioche@google.com>
IsNan returns a boolean, even is the input-type is a float. This was
working in most cases except:
- if the layout was not Void
- if the input type was not a matrix
The first bug is because a bool memory layout/representation is not
specified, and shall never be exposed to externaly-accessible memory.
Hence, if we saw a layout rule != Void, we converted it to a UINT. When
calling isnan, the layout rule should not be propagated as we loose any
layout info.
The second is because our codegen assumed matrix operations returned a
matrix with the same type as the input parameters. In the case of isnan,
this was just wrong.
Fixes#6712
Signed-off-by: Nathan Gauër <brioche@google.com>
There is an extra "signature" in the Patch Constant signature part of
the disassembler output, causing the "Patch Constant signature
signature" to appear. This PR removes the extra "signature" from the
output.
Example HLSL input:
```hlsl
struct OutputConstantData {
float tessFactor[4] : SV_TessFactor;
float insideTessFactor[2] : SV_InsideTessFactor;
};
OutputConstantData HSConstant() {
OutputConstantData output;
return output;
}
[domain("quad")]
[partitioning("integer")]
[outputtopology("triangle_cw")]
[outputcontrolpoints(1)]
[patchconstantfunc("HSConstant")]
void HSMain() {}
```
Example Disassembler output:
```diff
...
;
- ; Patch Constant signature signature:
+ ; Patch Constant signature:
;
; Name Index Mask Register SysValue Format Used
; -------------------- ----- ------ -------- -------- ------- ------
; SV_TessFactor 0 w 0 QUADEDGE float w
; SV_TessFactor 1 w 1 QUADEDGE float w
; SV_TessFactor 2 w 2 QUADEDGE float w
; SV_TessFactor 3 w 3 QUADEDGE float w
; SV_InsideTessFactor 0 w 4 QUADINT float w
; SV_InsideTessFactor 1 w 5 QUADINT float w
;
...
```
This imports upstream commit
54bff1522f
This fixes the following compiler error with clang 18.1.8 with mingw-w64
toolchain.
CFG.h:916:22: error: expected a qualified name after 'typename'
916 | template <typename CALLBACK>
| ^
minwindef.h:90:18: note: expanded from macro 'CALLBACK'
90 | #define CALLBACK __stdcall
| ^
This change adds a new CMake configuration option
`HLSL_DISABLE_SOURCE_GENERATION` which allows a user to disable
generating the in-tree sources which contributte to DXC's source
releases. This option should only be used by users building DXC and not
modifying it.
Resolves#6728
In the special code to handle the memcpy pattern where a constant buffer
contains a vector array that initializes a local (or static global)
scalar array for use by the shader, an invalid assumption was made that
if the memcpy dest was global, that the src is global as well.
This was not the case, and when expecting to generate constant
expressions to index the src, these generated orphaned instructions
instead, leading to invalid IR.
This fixes the issue by leveraging ReplaceConstantWithInst, and setting
the insertion point for the Builder. Now, replacement *could* fail, if
src instructions don't dominate replacement uses, so bool for replaced
all is returned from replaceScalarArrayWithVectorArray.
Another issue was that it would replace the dest for the original memcpy
with src along the way. Now, if we don't replace all uses, this turns
the memcpy into a no-op and any remaining uses are no longer coming from
src, but an undef dest instead. This was also fixed to skip this
replacement, then clean up this use if all other uses have been
successfully replaced.
Fixes#6510
The documentation says that we use the HitTKHR builtin to implement
RayTCurrent. However, HitTKHR was renamed to RayTMaxKHR. We update
the documentation to represent that change.
Fixes#6739
The behavior was changed with #6317 so that regardless of spelling in
the shader, the include path will conform to the host OS style for the
purposes of the include handler. This just adds a release note for that
new behavior.
Fixes#6669
Add guidance for how release notes should be documented at the time
of the change going in as well as some suggestions for how to format
that documentation.
Contributes to #6697
---------
Co-authored-by: Chris B <cbieneman@microsoft.com>
According to the HLSL semantics documentation, the valid semantic
indices for SV_Target[n] are 0 <= n <= 7:
https://learn.microsoft.com/en-us/windows/win32/direct3dhlsl/dx-graphics-hlsl-semantics
A check for this already exists in DXIL validation, but for large values
of n, crashes and/or buffer overruns may occur during compilation before
validation, so an earlier check is needed.
Fixes#6115
When enabled, any hlsl::Exception thrown during code generation and
optimization will cause the process to trap.
---
edit: I've changed the implementation to generate a trap, instead of
std::abort
Moving the source file for the README.md for github and nuget releases
into the DXC repo. It should be updated with relevant notes for the
upcoming release whenever they are made in main such that they will
already be available when the release is built.
Part of #6697
…list
Instructions in a BasicBlock are maintained in a doubly-linked list. The
links are "instrusive" in the sense that with inheritance tricks the
Next and Prev nodes are embedded in the Instruction object itself.
The linked list uses a sentinel object to mark the tail of the list.
Previously, the sentinel was an ilist_half_node<Instruction> which just
consists of a 'Instruction* Prev', but then it was cast to Instruction*.
This is flagged by the undefined behaviour sanitizer in some cases.
Also, it's weird and wrong.
Upstream LLVM has entirely reimplimented the intrusive list to avoid
such problems. But that's a massive change.
This change uses a real Instruction object as the sentinel. The
instruction is only used for its Next and Prev properties. I used an
UnreachableInst becuase it's small and simple.
Issue: #6446
The `ResElem` type doesn't exist, and the example here seemed to imply
that the similar `ResRet` type was used in resource metadata. This is
incorrect, so fix the examples to match types that would actually show
up in this metadata.
Note: In practice the metadata doesn't generally actually refer to a
variable, but just an `undef` of the right type. I've opted not to
change the examples to reflect that here to minimize the change, but it
might be nice to describe when/why this occurs.
Fixes#3411
This fixes the following compiler error with clang 18.1.6 with mingw-w64
toolchain.
microcom.h:190:43: error: unknown type name 'nullptr_t'; did you mean
'std::nullptr_t'?
190 | template <typename T> HRESULT AssignToOut(nullptr_t value, T
*pResult) {
| ^~~~~~~~~
| std::nullptr_t
microcom.h:207:43: error: unknown type name 'nullptr_t'; did you mean
'std::nullptr_t'?
207 | template <typename T> void AssignToOutOpt(nullptr_t value, T
*pResult) {
| ^~~~~~~~~
| std::nullptr_t
As we prepare to do a release, we generally create an issue that
contains instructions and checklists for the mechanics of the release.
This change adds a template for these issues.
This pass was attempting to compare different things. The return values
of GetDxilVersion are not shader models, but... dxil version. Since the
code is trying to upgrade the validator version, I changed this to
GetValidatorVersion, to pair with SetValidatorVersion.
The previous code was breaking the nvidia driver on workgraphs.
Internal build that has DXC as a submodule and that is built with a
different VC toolset version started failing after the pragma got moved
up in commit 0b9acdb75. Adding a duplicate pragma back at the original
location makes both compiler versions happy.
If an exception is thrown, don't block it in the TempOverloadPool
destructor. Allow it to propagate out as a user-visible error.
Explicitly clear the TempOverloadPool before returning from the
HLMatrixLowerPass::runOnModule. In the normal case, when no exception is
thrown, that will still verify that all the overloads actually have been
lowered, and will assert out if they aren't.
The first first fix in #5392 was not correct. It relied on the layout
rule for the address to be the correct layout rule, but that is not
always the case. The address is just an integer that could exist in any
storage class. The correct solution is to explicitly set the layout rule
for the BitCast operation when expanding the RawBuffer* functions. We
know that the result of the BitCast is a pointer to the physical storage
buffer storage class, so we know the layout need to be the storage
buffer layout.
Fixes#6554
Originally @lizhengxing's PR. Retargeting main.
This PR pulls the upstream change, Fix non-determinism in Reassociate
caused by address coincidences
(ef8761fd3b),
into DXC.
Here's the summary of the change:
Between building the pair map and querying it there are a few places
that erase and create Values. It's rare but the address of these newly
created Values is occasionally the same as a
just-erased Value that we already have in the pair map. These
coincidences should be accounted for to avoid non-determinism.
Thanks to Roman Tereshin for the test case.
This is part 6 (the last part) of the fix for #6659.
Co-authored-by: Zhengxing Li <zhengxingli@microsoft.com>
This PR pulls the following upstream changes into DXC:
[llc/opt] Add an option to run all passes twice
(04464cf731)
> Lately, I have submitted a number of patches to fix bugs that only
occurred when using the same pass manager to compile
> multiple modules (generally these bugs are failure to reset some
persistent state).
>
> Unfortunately I don't think there is currently a way to test that from
the command line. This adds a very simple flag to both
> llc and opt, under which the tools will simply re-run their respective
> pass pipelines using the same pass manager on (a clone of the same
module). Additionally, we verify that both outputs are
> bitwise the same.
>
> Reviewers: yaron.keren
[opt] Fix sanitizer complaints about r254774
(38707c45be)
> `Out` can be null if no output is requested, so move any access
> to it inside the conditional. Thanks to Justin Bogner for finding
> this.
This is for the test of this change
(ef8761fd3b)
to fix#6659.
---------
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: Adam Yang <31109344+adam-yang@users.noreply.github.com>
Originally @lizhengxing's PR. Retargeting main.
This PR pulls 2 upstream changes, Add a new WeakVH value handle; NFC
(f1c0eafd5b)
and Use a 2 bit pointer in ValueHandleBase::PrevPair; NFC
(b297bff1cc),
into DXC.
Here's the summary of the changes:
Add a new WeakVH value handle; NFC
> WeakVH nulls itself out if the value it was tracking gets deleted, but
it does not track RAUW.
>
> Reviewers: dblaikie, davide
>
> Subscribers: mcrosier, llvm-commits
>
> Differential Revision: https://reviews.llvm.org/D32267
Use a 2 bit pointer in ValueHandleBase::PrevPair; NFC
> This was an omission in r301813. I had made the supporting changes to
make this happen, but I forgot to actually update the
>
> PrevPair declaration.
This is part 4 and 5 of the fix for #6659.
Some of the types that have been added to the vk namespace were being
added to the default namespace when compiling for DXIL. The if
conditions were such that they would fall through to a default case.
The solution is to explicitly add code that we should skip adding those
builtin types when the vk namespace is not defined.
Fixes#6646.
This PR pulls the upstream change, Rename WeakVH to WeakTrackingVH; NFC
(e6bca0eecb),
into DXC.
Here's the summary of the change:
> I plan to use WeakVH to mean "nulls itself out on deletion, but does
not track RAUW" in a subsequent commit.
>
> Reviewers: dblaikie, davide
>
> Reviewed By: davide
>
> Subscribers: arsenm, mehdi_amini, mcrosier, mzolotukhin, jfb,
llvm-commits, nhaehnle
>
> Differential Revision: https://reviews.llvm.org/D32266
This is part 3 of the fix for #6659.
---------
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
When processing the GetDimension member function for textures, we do not
emit an error if the output variable is not an l-value. This change will
add this error.
Fixes#6689
This commit bumps SPIR-V tools version, and re-add support for objects
debug instructions when using Vulkan's debug instructions.
Because OpenCL debug instructions are not a non-semantic set, the SPIR-V
spec would need to be modified, as today it does not allows forward
references.
Fixes#6691
---------
Signed-off-by: Nathan Gauër <brioche@google.com>
Induction variable simplification (indvars) tries to rewrite exit
values; these appear as phi nodes in loop exit blocks. If the
replacement for the phi is still in the loop, then that would break the
LCSSA property. Don't do that.
Add a test for this.
We will start issues a warning when `vk::offset` is not correctly
aligned to make it easier for users to understand why their spir-v will
not validate. Note that we do not treat this as an error because we want
to allow someone to have the flexibility to do other things. For
example, they could be targeting an API that does not follow any of
the existing rules, which is why they are using `vk::offset`.
Fixes#6171
Previously, if the latch exit was reachable from a different exit block
for the loop, then the pass would introduce a loop involving that exit
block and the latch exit. This is unwanted and unaccounted for.
- Add a test for shared exits.
- Add a test for non-dedicated latch exit
Before this change, OpConstantNull was emitted when an undef value was
required.
This causes an issue for some types which cannot have the OpConstantNull
value.
In addition, it mixed well-defined values with undefined values, which
prevents any kind of optimization/analysis later on.
Fixes#6653
---------
Signed-off-by: Nathan Gauër <brioche@google.com>
Nathan Gauër
Chris B
gracezhang72
Greg Roth
Biswapriyo Nath
Steven Perron
Tianlan Zhou
Tex Riddell
Xiang Li
Natalie Chouinard
David Neto
Justin Bogner
dependabot[bot]
Raed Rizqie
Damyan Pepper
Jeff Noyle
Helena Kotas
Adam Yang
Zhengxing li