This is some preliminary work to bitfield addition into the SPIR-V
backend. This commit should not change the behavior of our backend.
Signed-off-by: Nathan Gauër <brioche@google.com>
Signed-off-by: Nathan Gauër <brioche@google.com>
Things like -DResType=RasterizerOrderedStructuredBuffer<uint> could be workaround with
-DResType="RasterizerOrderedStructuredBuffer<uint>" for lit.
But clang-hlsl-tests will not work.
So put the template inside the test.
* WinAdapter: Remove virtual dtors from IUnknown to fix vtable ABI
The vtable for `IUnknown` and its subclasses contain two deletion
pointers when compiled on non-Windows systems with `IUnknown` from
`WinAdapter.h`:
vtable for 'DxcLibrary' @ 0x7ffff7cbc5f8 (subobject @ 0x5555556bb9e0):
[0]: 0x7ffff6a56d40 <DxcLibrary::QueryInterface(_GUID const&, void**)>
[1]: 0x7ffff6a56d20 <DxcLibrary::AddRef()>
[2]: 0x7ffff6a56d30 <DxcLibrary::Release()>
[3]: 0x7ffff6b36bc0 <IUnknown::~IUnknown()> // Complete object destructor
[4]: 0x7ffff6a57130 <DxcLibrary::~DxcLibrary()> // Deleting destructor
[5]: 0x7ffff6a56d50 <DxcLibrary::SetMalloc(IMalloc*)>
[6]: 0x7ffff6a56d60 <DxcLibrary::CreateBlobFromBlob(IDxcBlob*, unsigned int, unsigned int, IDxcBlob**)>
... More DxcLibrary virtual functions
This shifts the the pointers for functions for all subclasses, and is
[annoying] to deal with in otherwise cross-platform applications using
DirectXShaderCompiler as library. `dxcompiler.dll` compiled on/for
Windows without `WinAdapter.h` does not suffer this problem, and only
has three function pointers for `IUnknown`.
Fortunately, it is easily solved by removing the virtual destructor from
`IUnknown`. LLVM enables `-Wnon-virtual-dtor` that warns against
classes with virtual methods but no virtual destructor, though this
warning is best not enabled akin to Windows builds where `IUnknown` from
`windows.h` (`unknwn.h`) results in the same warning on MSVC ([1]/[2]).
[annoying]: 1e624792fc/src/unknown.rs
[1]: https://github.com/microsoft/DirectXShaderCompiler/issues/3783#issuecomment-844189358
[2]: https://godbolt.org/z/hKPT6ThEf
* WinAdapter: Make `IUnknown` and `IMalloc` pure-virtual classes
`IUnknown` in Windows' `unknwn.h` and `IMalloc` in `ObjIdl.h` are marked
as pure virtual, and are best marked as such in `WinAdapter` for
non-Windows platforms too [1]. Only the shim for `IMalloc` was relying
on the default refcounting implementation, all other subclasses either
contain pure-virtual methods themselves or provide an implementation for
`AddRef`/`Release` as required. Likewise the default implementation for
`IMalloc` was only instantiated once by `CoGetMalloc`, and has been
moved into a local class implementing the `IMalloc` interface instead.
[1]: https://github.com/microsoft/DirectXShaderCompiler/pull/3793#issuecomment-846459741
* WinAdapter: Add three missing virtual functions to `IMalloc` interface
To prevent unexpected vtable breakage, add the missing functions from
the [documentation]. Note that they are listed in the wrong order, the
right order is retrieved from the `ObjIdl.h` header and implementations
for `IMalloc` in DirectXShaderCompiler. All implementations are now
properly using the `override` keyword too, to enforce virtual method
existence in the base class.
[documentation]: https://docs.microsoft.com/en-us/windows/win32/api/objidl/nn-objidl-imalloc
* Make all WinAdapter destructions explicit
This prevents warnings about non-virtual destructor usage that trip up
the Linux build. It represents status quo on Windows.
Co-authored-by: Greg Roth <grroth@microsoft.com>
This change alters how globallycoherent attributes are validated. We
will need similar changes for other HLSL type attributes as well.
Without this change, globallycoherent attributes are only allowed to
apply to declarations of builtin objects during parsing. Additionally
if applied to a non-UAV we catch that late (during code generation) and
report a different error there.
This patch reworks all of that. Instead, during parsing we allow verify
the attribute is applying to a UAV builtin type and issue a diagnostic
if applied to a non-UAV type. We also allow the attribute to be applied
to any dependent type.
If the attribute is applied to a dependent type, we verify that the
dependent type is resolved to a valid UAV type during instantation.
This change also reworks the existing test case for globallycoherent
diagnostics and deletes duplicated tests that were not adding meanigful
value.
Fixes 4583.
This change makes mixing bitfields and HLSL annotations an error. Since
the syntax for HLSL annotations and bitfields are ambiguous, this change
works around some issues with the ambiguity by disallowing mixing the
two on the same declaration.
Fixes#4686
In some cases, code using getAsStructureType() seemed to expect any user-
defined type to result in RecordType here, but only 'struct' types would,
leaving 'class' types to fail certain code paths. Some code paths had an
additional getAs<RecordType>() if the getAsStructureType() returned nullptr,
but at that point, why bother with getAsStructureType() in the first place?
This updates cases in CGHLSLMS.cpp that looked to be misusing getAsStructureType
to simply use getAs<RecordType>() instead. One case is with constructing
type annotations, where two branches are used and code is almost identical,
except skipping size return when a member is a resource was only in the struct
path. I think removing this separate path and checking for resource on any
RecordType makes sense here.
HLSL ternary operators that result in vector or matrix types need
special handling even if the condition is not a vector. This change
allows vector and matrix result types for ternary operators even if
matrix and vector conditions are not allowed.
The change works by generating an alloca before ternary blocks, and
terminating ternary blocks by writing the resulting matrices to the
alloca. The result of the ternary is then a load of the alloca'd matrix.
Fixes#4434
There are some other functions that are found in the IsNormal HLK test that are defined and declared in the DxcTestUtils files. This is bad because DxcTestUtils takes a dependency on LLVM. This PR gets rid of the rest of the functions that don't need to depend on an llvm include, but are necessary to allow IsNormal to function, and moves the functions to HLSLTestUtils.h. Hopefully after this PR, IsNormal will be able to run when copied to other repos.
These functions and all their dependent functions were moved to HLSLTestUtils.h:
-VerifyCompileOK
-DisassembleProgram
-AssembleToContainer
DxilMutateResourceToHandle would skip GEP if result type does not need to be mutated.
But then the GEP would not have setSourceElementType called with the new source type leaving invalid IR.
The fix makes sure the GEP is added to MutateValSet when skipping recursion.
Types no longer in use, potentially due to type translation, are still kept
around just because they are referenced in the type annotation metadata.
This change adds a method DxilModule::RemoveUnusedTypeAnnotations() that
removes any type annotations not used by resources or function arguments
or return types. Types outside this set should no longer need any type
annotations. This will get rid of extra unused types often left over in
final dxil before.
Currently, ResourceAttribute is only used to capture resource type information during CodeGen for the annotate handle generated during AddOpcodeParamForIntrinsic.
Going through a metadata node is totally unnecessary, just adding code and complexity.
This change just stores the DxilResourceProperties instead, renaming `*ResourceAttribute` methods to `*ResourceProperties`.
This change also emits the resource properties to new field annotaion metadata in validator version 1.8+ to preserve through serialization.
* [SPIRV] Support 16-bit Type Cast Intrinsics
This change adds support for 16-bit type cast intrinsics by routing
them into the same logic used by the 32-bit type casts. With this
change in place, asfloat16, asint16, and asuint16 are now translated
into OpBitcast in SPIR-V.
This change also includes new partial disassembly tests for each
function similar to the ones used for 32-bit casts.
* Update Comments in processIntrinsicsAsType
This commit updates the comments at the top of processIntrinsicsAsType
in order to indicate that this function also supports the 16-bit
variants of the type casting intrinsics.
There was a crash when CBuffer vector subscript uses a non-literal index but is replaced with Constant in the optimizer before CBuffer load is lowerd. The CBuffer lowering code expects the constant to be within bounds, since it's translating the GEP to extractvalue. The out of bounds subscript is not correct and should cause an error, but ONLY if it actually exists in the final DXIL.
This change adds a way to emit error that only get emitted if the associated code is not removed by the end of compilation. Instead of emitting an error right away, emit a poison value with line number and error message instead and have it used by the problematic code. If the problematic code is not removed by the end of compilation, then this poison value would also be there, and it's safe to emit a real error based on it.
A very strange sequence of copying the run line to a stack char array of
size 300, where it would not even null-terminate the line if it hit the
limit, seems totally unnecessary, since it's dealing with std::strings
in the first place.
This would truncate and pick up garbage from the stack in the stored run
line when length >= 300 characters - causing the test to fail, of course.
Perhaps some of this is an artifact of some older version of the code.
In any case, this change removes the extra copying step and simply uses
the entire std::string result from std::getline() instead.
On second thought, it's safer to default to the initial version, letting
code that writes to RDAT set strides for newer record versions it actually
uses, rather than breaking RDAT writing code at runtime when versioned
updates are added to the structures.
It's possible to get a value of a type `unsigned long` in HLSL, such as
the result of sizeof(), while the uint type is `unsigned int` instead.
This leads to the situation where no cast is inserted because the types
look identical when translated down to ArBasicKind, which can lead to an
assert during codegen, such as when passing sizeof() result to a function
call parameter.
This change preserves the original canonical type ptr from clang and
sets the conversion type when these differ, even when ArBasicKind is
identical.
Added MaxRecordSize calling DerivedRecordSize that gets specialized for each base in RDAT_STRUCT_TABLE_DERIVED macro to call the derived MaxRecordStride. The most-derived version of DerivedRecordSize will use the default implementation, which is the size of that type.
* [SPIR-V] Add support for logical operator intrinsics
Implemented `and`, `or`, and `select`. I used the existing tests for the
old behavior of `&&`, `||`, and `?:` in order to verify that they behave
as expected (with the caveat of #4673, but that has the same behavior
for DXIL as for SPIR-V).
Fixes#4148
* Clean up
* Remove unused variable
I added this upstream a few years ago so that you can control passing
`-O1` to the compiler when building debug sanitized builds. Without
being able to disable optimizing sanitizer builds it is really tricky to
debug sanitizer failures, but without optimizing the build ASan builds
take _forever_ to run tests which makes them difficult to use.
- Added new PDBI part for PDBs to eventually replace SRCI
- Added new IDxcPdbUtils2 that can read PDBI
- Removed all functionalities to do with GetFullPDB (and its tests)
- Pulled out RDAT writing from DxilContainerAssembly into its own helper DxilRDATBuilder
- Changed the way RawBytesPart deduplicates inputs. Instead of using the pointer address, use the data's content as keys in a hash table, then writing it to the final buffer in the order they were inserted.
Nathan Gauër
Xiang Li
Cassandra Beckley
Jeff Noyle
Helena Kotas
Adam Yang
Paul Dodzweit
Diego Novillo
Marijn Suijten
Chris B
Tex Riddell
Joshua Batista
gmitrano-unity
Yuriy O'Donnell
Daniele Vettorel
Adam Yang
Greg Fischer