- Move validator/dxil version checks up-front
These should fail first rather than side effects of trying to validate
details of a version we don't support.
- Improve message for unsupported validator or dxil version
These errors are most likely if compiling separately from validation
and failing to override the validator version properly, or running on
an external validator that doesn't support a newer dxil.
- Use dxil version from metadata for DxilModule when loading,
rather than just setting it to minimum based on shader model.
- Remove TODO from validator messages that shouldn't be there
Gather operations that take four separate offsets are meant to allow for
programmable, non-immediate values. This removes them from the immediate
and range validations by giving the immediate versions their own opcodes
For these intrinsics and loads, errors are generated when offsets are non-literal
or out of range. These largely use the slightly altered validation paths that the
sample intrinsics use.
Reword error when texture access offsets are not immediates to be
more all encompassing and grammatical.
Incidentally remove duplicate shaders being used for the validation
test from the old directory while identical copies in the validation
directory went unused. Redirected validation test to the appropriate
copies. This is consistent with the test shader re-org's stated intent
Sample operations have a maximum of 3 args, but gather has a maximum of
two since it always operates on 2D images. So gather operations only
pass in two offset args to the offset validation, which resulted in an
invalid access.
Rather than adding a specialized condition to evade this, just iterate
over the number of elements in the array. For sample it will be 3 and
for gather 2 and it will still check for expected undefined args
appropriately.
For the offset legalization pass, the opcode is used to determine the
start and end of the offset args
Only produce the loop unroll suggestion when within a loop
Base error line on call instruction instead of source of the offset
Sort by location in source when possible and remove duplicates
Adapt tests to verify and match these changes
Fixes#2590Fixes#2713
According to Vulkan specification when using `OpImageRead/OpImageWrite`, the `OpTypeImage` (`Buffers`, `RWBuffers`, `RWTextures`) must have a format that matches the format on the API side, unless the StorageImageReadWithoutFormat/StorageImageWriteWithoutFormat is added and `Unknown` is used as the format.
This pull request addressess #2498 for the format part by adding an attribute `[[vk::image_format("<image format as spelled in SPIR-V spec>")]].` Example of the syntax:
```
[[vk::image_format("rgba8")]]
RWBuffer<float4> Buf;
[[vk::image_format("rg16f")]]
RWTexture2D<float2> Tex;
RWTexture2D<float2> Tex2; // Works like before
```
The `image_format` only applies to **global variables** of type `Buffer`, `RWBuffer`, `RWTexture`. For variables and function parameters it is propagated by the inlining pass in legalization. This required a small change to one of the passes in SPIRV-Tools, that should be also checked by someone more familiar with the codebase: https://github.com/KhronosGroup/SPIRV-Tools/pull/4126
Note that this does not fix the handling of unspecified format (that case still works like before, using `R32f`, etc. based on the type in shader), although it should be still fixed to add the
StorageImageReadWithoutFormat and/or StorageImageWriteWithoutFormat and use Undefined. But I think the ability to specify the format is more urgent.
Design note from Jaebaek:
Since the `image_format` attribute only applies to **global variables**, under the DXC architecture
only `DeclResultIdMapper` can check the attribute when it handles `VarDecl`s. It means
we have to pass the `image_format` information to `LowerTypeVisitor` because it cannot access to
`VarDecl`. In order to pass the `image_format`, we use `SpirvContext` that can be accessed by
`SpirvEmitter` and all visitors. We use `SpirvVariable` to `spv::ImageFormat` mapping because the
attribute only applies to **global variables** (not to image types).
See how we use `llvm::DenseMap<const SpirvVariable *, spv::ImageFormat> spvVarToImageFormat`.
groupshared variables shouldn't be allowed outside of compute and
compute-like shaders. This adds a validation error when such
variables are used.
Add shader mask where groupshared are used
Correct several tests that used groupshared incorrectly
Incidental fix to atomics tests where the special case initializations
might have allowed the threads that failed to write to proceed
before the write took place and read the improperly initialized values
Changed up the assignments to avoid write collisions and be more
readable
Fixes#2603, #2677
In DX12, `SV_InstanceID` always counts from 0. On the other hand,
Vulkan `gl_InstanceIndex` starts from the first instance. Thus it
doesn't emulate actual DX12 shader behavior. To make it equivalent,
we can set `SV_InstanceID = gl_InstanceIndex - gl_BaseInstance`.
However, it can break the existing working shaders (i.e., compatibility
issue). We want to provide an option for users to choose what they
exactly want by `SV_InstanceID`. This commit adds
`-fvk-support-nonzero-base-instance` option. If it is enabled, it emits
`SV_InstanceID = gl_InstanceIndex - gl_BaseInstance`. Otherwise,
`SV_InstanceID = gl_InstanceIndex`.
This is the work of many contributors from Microsoft and our partners.
Thank you all!
Add support for 64-bit atomics, compute shader derivatives, dynamic
resources (from heap), 8-bit Packed Operations, and Wave Size.
All of these require compiling with 6_6 targets with just a few
exceptions. Each of these features include unittests and Execution
tests.
64-bit atomics add 64-bit variants of all Interlocked* intrinsic
operations. This involves changing some of the code that matches
intrinsic overloads to call instructions. Also adds a few float
intrinsics for compare and exchange interlocked ops which are available
for all shader models 6.0 and up.
Compute shader derivatives adds dd[x|y], CalculateLevelOfDetail, and
Sample operations that require derivatives to compute. QuadRead
operations have been allowed in compute from 6.0+ and tests are added
for them here.
Dynamic resources introduce global arrays that represent the resource
and sampler heaps that can be indexed without requiring root signature
representations. This involves a new way of creating and annotating
resource handles.
8-bit Packed operations introduces a set of intrinsics to pack and
unpack 8-bit values into and out of new 32-bit unsigned types that can
be trivially converted to and from uints.
WaveSize introduces a shader attribute that indicates what size the
shader depends on the wave being. If the runtime has a different wave
size, trying to create a pipeline with this size will fail.
* [spirv] Introduce the implicit 'vk' namespace
If any of these are used for DXIL code generation, the compiler will
report an error about the unknown "vk" namespace.
* [spirv] Introduce vk::ReadClock intrinsic.
The following Vulkan specific intrinsic functions are added:
```hlsl
uint64_t vk::ReadClock(in uint32 scope);
```
Also the following Vulkan-specific implicit constants are added:
```
vk::CrossDeviceScope; // defined as uint 0
vk::DeviceScope // defined as uint 1
vk::WorkgroupScope // defined as uint 2
vk::SubgroupScope // defined as uint 3
vk::InvocationScope // defined as uint 4
vk::QueueFamilyScope // defined as uint 5
```
Sample usage looks as follows:
```hlsl
uint64_t clock = vk::ReadClock(vk::WorkgroupScope);
```
If any of these are used for DXIL code generation, the compiler will
report an error about the unknown "vk" namespace.
* [spirv] Add documentation.
* Address code review comments.
* Test: Validate vk namespace is not allowed for dxil.
* Fix usage of DXASSERT.
* Move ValidateVkNamespaceNotAllowed test to HlslFileCheck.
* Add -fvk-auto-shift-bindings that applies -fvk-*-shift bindings to resources that do not have explicit register assignments. Closes#2956
* For resources that don't have an explicit register binding, first categorize their registerType using new method DeclResultIdMapper::getImplicitRegisterType.
* When finding automatic binding indices for implicit register bindings, pass an optional shift to useNextBinding.
* Change getNextBindingChunk() to handle finding the next binding range given a bindingShift
* Fix regression with binding numbers causing AppVeyor failure and also fix crash I ran into in my own testing in getImplicitRegisterType() if the getSpirvInstr() did not have an AstResultType.
* Address review comments.
* Update documentation
Co-authored-by: Ehsan Nasiri <ehsannas@gmail.com>
The validation error message for when one or more of the components of a
storeBuffer (or similar) operation are undefined was terribly confusing
to anyone not developing the compiler. It mentioned mask mismatches when
one of the masks was generated based on which operands were undefined.
This changes the error message when the flaw results from the user under-
specifying the copied variable to simply state that the assignment involves
undefined values. If the masks mismatch because the type-based mask didn't
expect values that the user somehow defined, the original error message
results as this is likely a compiler error.
In addition, this consolidates some of the mask checking for rawbuffer,
texture, and generic buffer storage.
This strips the previous method of error reporting from DxilValidation
in favor of the DxilUtil methods. With these comes a new way to find
location information for those associated with functions or globals.
Instruction reporting is slightly changed in that the asm informtion
including the instruction and its block is added as a note so as to not
clutter up the line with the error and hlsl location.
For the most part, the existing EmitError* functions were retained, but
their implementation was changed to largely format inputs for the
dxilutil:: function and call it.
Errors that had clear associations with instructions, functions, or
globals had their emitting functions changed to variants that had this
information. The result is error message with as much location
information as is available.
In some cases, functions that implicitly added format arguments where
removed in favor of explicit formatted error calls that are clearer.
* Avoid directory duplication in DIFile MD
Change #2030 prevented the duplication of the directory for filenames
with full paths. However the same problem exists for relative paths that
include directories. This results in duplicate DIFile entries and
confusing error message outputs for Global Variables.
This skips the original code which was meant to prepend the directory
name since our filenames always have that information already.
* Validation error message text changes
This includes a number of different changes to validation error
messages. Some correct grammar. Some add a format argument to the
message for better information. Many of them just add a period to the
end of the error message for consistency and to allow the "Use /Zi"
message to be tagged on the end without jumbling the output.
* Enhancements for Dxil DiagnosticInfo and DxilUtil
Introduce *OnContext diagnostic reporting for messages that don't
clearly adhere to any instruction, global, or function. Also includes
the ability to add a note to supplement an earlier diagnostic.
Add function to DiagnosticInfo. This provides a fallback option for
printing location information when debug info isn't available. At least
we can print the function where the issue occurs.
For GV errors, we can't depend on the module having a dxil module in it.
Debug modules don't get the dxil part cloned. The only use of it was the
potentially cached debuginfofinder. Lacking it, we can just create it on
the fly.
Corrected the format of the output for DiagnosticInfoDxil::print. In
addition to minor fixups, when the diagnostic is a remark or note, it
doesn't need the "Use /Zi" prompt.
DxilLoopUnroll has its own diagnostics reporting. This adds the
associated function for the messages.
* Add function location information for interp mode error
This is to satisfy a specific request for line information for this
specific error. It got left out of the original batch, but it's
important enough for a followon.
`@dx.op.bufferLoad` is for `BufferLoad`, not `RawBufferLoad`.
Similar to the previous, the appropriate function for `RawBufferStore` is `@dx.op.rawBufferStore`.
This matches what DxilOperations.cpp does.
Add GetResourceFromHeap for hlsl.
For Dxil, add CreateHandleFromHeap and AnnotateHandle.
All handles ( createHandle, createHandleForLib, createHandleFromHeap ) must be annotated with AnnotateHandle before use.
TODO: add AnnotateHandle for pix passes.
cleanup code about resource.
New Rules:
outer type may be: [ptr to][1 dim array of]( UDT struct | scalar )
inner type (UDT struct member) may be: [N dim array of]( UDT struct | scalar )
scalar type may be: ( float(16|32|64) | int(16|32|64) )
- Disallow pointers to pointers, and pointers in structs
- Disallow multi-dim arrays at top-level, but allow within struct
Two test options, -Qstrip_reflect_from_dxil and -Qkeep_reflect_in_dxil
for making tests work with reflection removed, since many tests are relying
on main module disassembly-reassembly between test phases and reflection
metadata will no longer be present there. The strip option is for the
few cases where tests don't want the reflection kept in DXIL by default.
Validator no longer requires function annotations for no reason.
Fix places where remove global hook was not being called when functions
were removed manually from the list.
StripReflection now deletes function annotations, unless targeting lib or
old validator that required them. Preserve global constructor list and
add annotation for 1.4 validator. The global hook fixes were required
here, otherwise annotations would refer to dead functions during linking.
Struct annotations may not be removed in library case when they still need
translation to legacy types.
Allow missing struct annotation when not necessary to upgrade the layout.
Preserve usage in reflection by upgrading the module, emitting metadata,
cloning for reflection, then restoring validator version and re-emit
metadata.
Fix size for 16-bit type for usage and reflected size.
Make various batch reflection tests require validator 1.5, since these
tests rely on module disassembly->assembly, which will not preserve extra
usage metadata for reflection in 1.4.
Include reflection part in IDxcAssembler, but don't strip from module,
since there are no options to prevent this from breaking a lot of tests.
Don't strip reflection from offline lib target.
* [spirv] Add option to flatten array of resources.
Current SPIR-V code generation uses 1 binding number for an array of
resources (e.g. an array of textures). However, the DX side uses one
binding number per array element. The newly added
'-fspv-flatten-resource-arrays' changes the SPIR-V backend behavior to
use one binding number per array element, and uses spirv-opt to flatten
the array.
TODO: Add a test where the array is passed around.
TODO: Test this works with steven's PR and proper results are produced.
* [spirv] Update tests to include array of samplers.
* [spirv] Take early exit condition out of the loop.
* [spirv] Add documentation for the new cmd option.
* [spirv] Invoke CreateDescriptorScalarReplacementPass when needed.
* [spirv] address code review comments.
- Compute mesh payload size before final object serialization
- During CodeGen for MS based on payload parameter
- During CollecShaderFlagsForModule for AS based on DispatchMesh call
- Store payload sizes in corresponding funtion properties, serializing
these properly for HL and Dxil Modules
- Use payload sizes from function props for PSV0 data during serialization
- Validate measured and declared payload sizes, don't just fill in
properties during validation
- Fix Wave/Quad allowed shader stages, enabling Quad* with CS-like models
- rename payloadByteSize members to payloadSizeInBytes
- Add GetMinShaderModelAndMask overload taking CallInst for additional
detail required to produce correct SM mask for Barrier operations
- Update the HLSL syntax from FeedbackTexture2DMinLod to FeedbackTexture2D<SAMPLER_FEEDBACK_MIN_MIP>
- Update DXIL to only have two UAV types for FeedbackTexture2D[Array] and use an extra metadata field to distinguish between the sampler feedback type.
- base of struct should always be aligned - or internal bug
- offset for array member must always be aligned - (new) validation error
- alloc and verify struct layouts even when not array field
- out of bound check would have missed OOB on last array element
* [spirv] Respect the -auto-binding-space option in SPIR-V backend.
* [spirv] Use default space if vk::binding doesn't provide dset.
* Address code review comments.
Tex Riddell
Greg Roth
seppala2
Jaebaek Seo
Frank de Jong
Vishal Sharma
Ehsan
JiaoluAMD
Dan Ginsburg
Maciej Latocha
JiaoluAMD
Erik Faye-Lund
alelenv
Xiang Li
amarpMSFT
Sahil Parmar
czw831024
Tristan Labelle
Amar Patel (GRAPHICS)
Sahil Parmar
JasperNV