Fix code quality issues reported by static analysis tool, such as:
- Rule of Three/Five.
- Dereference after null check.
- Unchecked return value.
- Variable copied when it could be moved.
Prior to this patch, when `NumElems` was 0, `OpTypeRuntimeArray` was
directly generated, but it requires `Shader` capability, so it can only
be generated if `Shader` env is being used. We have observed a pattern
of using unbound arrays that translate into `[0 x ...]` types in OpenCL,
which implies `Kernel` capability, so `OpTypeRuntimeArray` should not be
used. To prevent this scenario, this patch simplifies GEP instructions
where type is a 0-length array and the first index is also 0. In such
scenario, we effectively drop the 0-length array and the first index.
Additionally, the newly added test prior to this patch was generating a
module with both `Shader` and `Kernel` capabilities at the same time,
but they're incompatible. This patch also fixes that.
Finally, prior to this patch, the newly added test was adding `Shader`
capability to the module even with the command line flag
`--avoid-spirv-capabilities=Shader`. This patch also has a fix for that.
In Vulkan, the signedness of the accesses to images has to match the
signedness of the backing image.
See
https://docs.vulkan.org/spec/latest/chapters/textures.html#textures-input,
where it says the behaviour is undefined if
> the signedness of any read or sample operation does not match the
signedness of the image’s format.
Users who define say an `RWBuffer<int>` will create a Vulkan image with
a signed integer format. So the HLSL that is generated must match that
expecation.
The solution we use is to generate a `spirv.SignedImage` target type for
signed integer instead of `spirv.Image`. The two types are otherwise the
same.
The backend will add the `signExtend` image operand to access to the
image to ensure the image is access as a signed image.
Fixes#144580
The SPIR-V backend does not have access to the original name of a
resource in the source, so it tries to create a name. This leads to some
problems with reflection.
That is why start to pass the name of the resource from Clang to the
SPIR-V backend.
Fixes#138533
A new test added for spirv-friendly builtins for
SPV_KHR_bit_instructions unveiled that current mechanism to detect
whether SPIRV Backend is in OpenCL environment or Vulkan environment was
not good enough. This PR updates how to detect the environment and all
the tests accordingly.
*UPDATE*: the new approach is having a new member in `SPIRVSubtarget` to
represent the environment. It can be either OpenCL, Kernel or Unknown.
If the triple is explicit, we can directly set it at the creation of the
`SPIRVSubtarget`, otherwise we just leave it unknown until we find other
information that can help us set the environment. For now, the only
other information we use to set the environment is `hlsl.shader`
attribute at `SPIRV::ExecutionModel::ExecutionModel
getExecutionModel(const SPIRVSubtarget &STI, const Function &F)`. Going
forward we should consider also specific instructions that are
Kernel-exclusive or Shader-exclusive.
---------
Co-authored-by: marcos.maronas <mmaronas@smtp.igk.intel.com>
These are identified by misc-include-cleaner. I've filtered out those
that break builds. Also, I'm staying away from llvm-config.h,
config.h, and Compiler.h, which likely cause platform- or
compiler-specific build failures.
This relands #141031
This change ensures generated SPIR-V is valid and passes machine
verification:
```
*** Bad machine code: inconsistent constant size ***
- function: foo
- basic block: %bb.1 entry (0x9ec9298)
- instruction: %12:iid(s8) = G_CONSTANT i4 1
```
That is done by promoting `G_CONSTANT` instructions with small integer
types (e.g., `i4`) to `i8` if no extensions for "special" integer types
are enabled.
Packed structs get a different layout than a struct that is not packed.
This is handled by assigning different offset decoration when
appropriate. The `CPacked` decoration is not required, and is in fact
not valid when creating a shader.
Similaraly the alignment decoration is not allows when creating a
shader. We must avoid generating that decoration.
Fixes https://github.com/llvm/llvm-project/issues/138268
The asan failure was fixed by #138695, but another failure was
introduced in the meantime. The cause for the other failure has been
fixed. I will reapply the two PRs.
Reapply "[SPIRV] Add explicit layout (#135789)"
This reverts commit 0fb5720b4bf461d4d51ee85a8a6f4ea4f6fb4966.
Reapply "[SPIRV] Fix asan failure (#138695)"
This reverts commit df90ab96fb5a10df88fcfe6b0e8e63781ca24eca.
When the DataLayout is destroyed, the memory backing `Offsets` is
released. This causes a use after free.
To fix it, I added a DataLayout varible that will not be destroyed until
after Offsets is used.
Fixes asan failure caused by
https://github.com/llvm/llvm-project/pull/135789
Adds code to add offset decorations when needed. This could cause a
type mismatch for memory instructions. We add code to fix up OpLoad
instructions, so that we could get some tests. Other memory operations
will be handled in another PR.
Part of https://github.com/llvm/llvm-project/issues/134119.
This implements the result of the discussion at:
https://discourse.llvm.org/t/rfc-report-fatal-error-and-the-default-value-of-gencrashdialog/73587
There are two different use cases for report_fatal_error, so replace it
with two functions reportFatalInternalError() and
reportFatalUsageError(). The former indicates a bug in LLVM and
generates a crash dialog. The latter does not. The names have been
suggested by rnk and people seemed to like them.
This replaces a lot of the usages that passed an explicit value for
GenCrashDiag. I did not bulk replace remaining report_fatal_error usage
-- they probably require case by case review for which function to use.
When the SPV_INTEL_arbitrary_precision_integers extension is allowed to
be used, the backend will unconditionnally add it to the module used
extensions.
The patch prevent SPV_INTEL_arbitrary_precision_integers from being
declared if unneeded.
Adds code to expand the `llvm.spv.resource.handlefrombinding` and
`llvm.spv.resource.getpointer` when the resource type is
`spirv.VulkanBuffer`.
It gets expanded as a storage buffer or uniform buffer denpending on the
storage class used.
This is implementing part of
https://github.com/llvm/wg-hlsl/blob/main/proposals/0018-spirv-resource-representation.md.
- Use static instead of anonymous namespace for file local functions.
- Enclose file-local classes in anonymous namespace.
- Eliminate `llvm::` qualifier when file has `using namespace llvm`.
- Eliminate namespace surrounding entire code in
SPIRVConvergenceRegionAnalysis.cpp file.
- Eliminate call to `initializeSPIRVStructurizerPass` from the pass
constructor (https://github.com/llvm/llvm-project/issues/111767)
This PR is to thoroughly rework duplicate tracker implementation and
tracking of IR entities and types. These are legacy parts of the project
resulting in an extremely bloated intermediate representation and
computational delays due to inefficient data flow and structure choices.
Main results of the rework:
1) Improved compile-time performance. The reference binary LLVM IR used
to measure speed gains in
https://github.com/llvm/llvm-project/pull/120415 shows ~x5 speed up also
after this PR. The timing before this PR is ~42s and after this PR it's
~7.5s. In total this PR and the previous overhaul of the module analysis
in https://github.com/llvm/llvm-project/pull/120415 results in ~x25
speed improvement.
```
$ time llc -O0 -mtriple=spirv64v1.6-unknown-unknown _group_barrier_phi.bc -o 1 --filetype=obj
real 0m7.545s
user 0m6.685s
sys 0m0.859s
```
2) Less bloated intermediate representation of internal translation
steps. Elimination of `spv_track_constant` intrinsic usage for scalar
constants, rework of `spv_assign_name`, removal of the gMIR `GET_XXX`
pseudo code and a smaller number of generated `ASSIGN_TYPE` pseudo codes
substantially decrease volume of data generated during translation.
3) Simpler code and easier maintenance. The duplicate tracker
implementation is simplified, as well as other features.
4) Numerous fixes of issues and logical flaws in different passes. The
main achievement is rework of the duplicate tracker itself that had
never guaranteed a correct caching of LLVM IR entities, rarely and
randomly returning stale/incorrect records (like, remove an instruction
from gMIR but still refer to it). Other fixes comprise consistent
generation of OpConstantNull, assigning types to newly created
registers, creation of integer/bool types, and other minor fixes.
5) Numerous fixes of LIT tests: mainly CHECK-DAG to properly reflect
SPIR-V spec guarantees, `{{$}}` at the end of constants to avoid
matching of substrings, and XFAILS for `SPV_INTEL_long_composites` test
cases, because the feature is not completed in full yet and doesn't
generate a requested by the extension sequence of instructions.
6) New test cases are added.
Spec can be found here https://github.com/intel/llvm/pull/15225
TODO for future patches:
- During spec review need to decide whether only FunctionCall or Atomic
instructions can be decorated and if not - move the code around adding
handling for other instructions;
- Handle optional string metadata;
- Handle LLVM atomic instructions;
- Handle SPIR-V friendly atomic calls returning via sret argument.
Signed-off-by: Sidorov, Dmitry <dmitry.sidorov@intel.com>
OpenCL is allowed to cast pointers, meaning they can resolve some type
mismatches this way. In logical SPIR-V, those are restricted. This new
pass legalizes such pointer cast when targeting logical SPIR-V.
For now, this pass supports 3 cases we witnessed:
- loading a vec3 from a vec4*.
- loading a scalar from a vec*.
- loading the 1st element of an array.
---------
Co-authored-by: Steven Perron <stevenperron@google.com>
The SPIR-V Backend uses the same set of utility functions, mostly though
not entirely from SPIRVGlobalRegistry, to generate gMIR and SPIR-V
opcodes, depending on the current stage of translation. This is
controlled by an explicit EmitIR flag rather than the current
translation pass, and there are legacy pieces of code where the EmitIR
flag is declared so that it has a default true value, allowing using
utility functions without explicitly declaring their intent to work
either in gMIR or in SPIR-V part of the lowering process.
While it may be ok to leave this default EmitIR flag as is in generation
of scalar integer/float types, as we don't expect to see any dependent
opcodes derived from such OpTypeXXX instructions, using of EmitIR by
default in aggregation types is a source of hidden logical flaws and
actual issues.
This PR provides a partial fix to the problem by removing default status
of EmitIR, requiring a user call site to explicitly announce its intent
to generate gMIR or SPIR-V code, fixes several cases of misuse of
EmitIR, and, the most important, fixes a nasty logical error that breaks
passing of actually asked EmitIR value by the default value in the
middle of the chain of calls, in the `findSPIRVType` call. The latter
error was a source of issues in the post-instruction selection pass that
has been getting gMIR code where SPIR-V was explicitly requested due to
overloaded with default parameters internal API in SPIRVGlobalRegistry
(most notably, `findSPIRVType`).
The code pattern that clang will generate for HLSL has changed from the
original plan. This allows the SPIR-V backend to generate code for the
current code generation.
It looks for patterns of the form:
```
%1 = @llvm.spv.resource.handlefrombinding
%2 = @llvm.spv.resource.getpointer(%1, index)
load/store %2
```
These three llvm-ir instruction are treated as a single unit that will
1. Generate or find the global variable identified by the call to
`resource.handlefrombinding`.
2. Generate an OpLoad of the variable to get the handle to the image.
3. Generate an OpImageRead or OpImageWrite using that handle with the
given index.
This will generate the OpLoad in the same BB as the read/write.
Note: Now that `resource.handlefrombinding` is not processed on its own,
many existing tests had to be removed. We do not have intrinsics that
are able to use handles to sampled images, input attachments, etc., so
we cannot generate the load of the handle. These tests are removed for
now, and will be added when those resource types are fully implemented.
This PR is to address legacy issues with module analysis that currently
uses a complicated and not so efficient approach to trace dependencies
between SPIR-V id's via a duplicate tracker data structures and an
explicitly built dependency graph. Even a quick performance check
without any specialized benchmarks points to this part of the
implementation as a biggest bottleneck.
This PR specifically:
* eliminates a need to build a dependency graph as a data structure,
* updates the test suite (mainly, by fixing incorrect CHECK's referring
to a hardcoded order of definitions, contradicting the spec requirement
to allow certain definitions to go "in any order", see
https://registry.khronos.org/SPIR-V/specs/unified1/SPIRV.html#_logical_layout_of_a_module),
* improves function pointers implementation so that it now passes
EXPENSIVE_CHECKS (thus removing 3 XFAIL's in the test suite).
As a quick sanity check of whether goals of the PR are achieved, we can
measure time of translation for any big LLVM IR. While testing the PR in
the local development environment, improvements of the x5 order have
been observed.
For example, the SYCL test case "group barrier" that is a ~1Mb binary IR
input shows the following values of the naive performance metric that we
can nevertheless apply here to roughly estimate effects of the PR.
before the PR:
```
$ time llc -O0 -mtriple=spirv64v1.6-unknown-unknown _group_barrier_phi.bc -o 1 --filetype=obj
real 3m33.241s
user 3m14.688s
sys 0m18.530s
```
after the PR
```
$ time llc -O0 -mtriple=spirv64v1.6-unknown-unknown _group_barrier_phi.bc -o 1 --filetype=obj
real 0m42.031s
user 0m38.834s
sys 0m3.193s
```
Next work should probably address Duplicate Tracker further, as it needs
analysis now from the perspective of what parts of it are not necessary
now, after changing the approach to implementation of the module
analysis step.
This PR fixes https://github.com/llvm/llvm-project/issues/120078 and
improves/simplifies parsing of demangled function name that aims to
detect a tip for floating point decorations. The latter improvement
fixes also a complaint from `LLVM_USE_SANITIZER=Address`.
This PR adds the following features:
* saturation and float rounding mode decorations,
* arithmetic constrained floating-point intrinsics (strict_fadd,
strict_fsub, strict_fmul, strict_fdiv, strict_frem, strict_fma and
strict_fldexp),
* and SPV_INTEL_float_controls2 extension,
* using recent improvements of emit-intrinsics step, this PR also
simplifies pre- and post-legalizer steps and improves instruction
selection.
This PR improves general validity of emitted code between passes due to
generation of `TargetOpcode::PHI` instead of `SPIRV::OpPhi` after
Instruction Selection, fixing generation of OpTypePointer instructions
and using of proper virtual register classes.
Using `TargetOpcode::PHI` instead of `SPIRV::OpPhi` after Instruction
Selection has a benefit to support existing optimization passes
immediately, as an alternative path to disable those passes that use
`MI.isPHI()`. This PR makes it possible thus to revert
https://github.com/llvm/llvm-project/pull/116060 actions and get back to
use the `MachineSink` pass.
This PR is a solution of the problem discussed in details in
https://github.com/llvm/llvm-project/pull/110507. It accepts an advice
from code reviewers of the PR #110507 to postpone generation of OpPhi
rather than to patch CodeGen. This solution allows to unblock
improvements wrt. expensive checks and makes it unrelated to the general
points of the discussion about OpPhi vs. G_PHI/PHI.
This PR contains numerous small patches of emitted code validity that
allows to substantially pass rate with expensive checks. Namely, the
test suite with expensive checks set ON now has only 12 fails out of 569
total test cases.
FYI @bogner
This PR is to fix the following issues:
* the SPIR-V Backend didn't generate Alignment decoration for alloca
instructions,
* we need to use types from demangled function declarations to specify
types for opaque pointers.
Goals of the PR are:
* to ensure that correct types are applied to virtual registers which
were used as return values in call lowering. A reproducer is attached as
a new test case, before the PR it fails because spirv-val considers
output invalid due to wrong result/operand types in OpPhi's;
* improve type inference by speeding up postprocessing of types: by
limiting iterations by checking what remains to process, and processing
each instruction just once for any number of operands with uncomplete
types;
* improve type inference by more accurate work with uncomplete types
(pass uncomplete property to dependent operands, ensure consistency of
uncomplete-types data structure);
* change processing order and add traversing of PHI nodes when type
inference apply instructions results to specify/update/cast operands
type (fixes an issue with OpPhi's result type mismatch with operand
types).
This commit adds an intrinsic that will read from an image buffer. We
chose to match the name of the DXIL intrinsic for simplicity in clang.
We cannot reuse the existing openCL readimage function because that is
not a reserved name in HLSL.
I considered trying to refactor generateReadImageInst, so that we could
share code between the two implementations. However, most of the code in
generateReadImageInst is concerned with trying to figure out which type
of image read is being done. Once we factor out the code that will be
common, then we end up with just a single call to the MIRBuilder being
common.
Implements elementwise firstbithigh hlsl builtin.
Implements firstbituhigh intrinsic for spirv and directx, which handles
unsigned integers
Implements firstbitshigh intrinsic for spirv and directx, which handles
signed integers.
Fixes#113486Closes#99115
When trying to create a const inst from a 32 bit signed value, we don't
want to sign-extend it to 64 bits, as the resulting value won't actually
fit in an `i32` if it was negative.
This fixes crashes in the following two tests after the APInt
constructor asserts were enabled in #114539:
```
Failed Tests (2):
LLVM :: CodeGen/SPIRV/transcoding/RelationalOperators.ll
LLVM :: CodeGen/SPIRV/uitofp-with-bool.ll
```
Implement the intrinsic `llvm.spv.handle.fromBinding`, which returns the
handle for a global resource. This involves creating a global variable
that matches the return-type, set, and binding in the call, and
returning the handle to that resource.
This commit implements the scalar version. It does not handle arrays of
resources yet. It also does not handle storage buffers yet. We do not
have the type for the storage buffers designed yet.
Part of #81036
The SPIRV backend has a special type named `spirv.Image`. This type is
meant to correspond to the OpTypeImage instruction in SPIR-V, but there
is one difference. The access qualifier operand in OpTypeImage is
optional. On top of that, the access qualifiers are only valid for
kernels, and not for shaders.
We want to reuse this type when generating shader from HLSL, but we
can't use the access qualifier. This commit make the access qualifer
optional in the target extension type.
The same is done for `spirv.SampledImage`.
Contributes to #81036
This PR reworks implementation of OpSpecConstantOp with ptr-cast
operation (PtrCastToGeneric, GenericCastToPtr). Previous implementation
didn't take into account a lot of use cases, including multiple
inclusion of pointers, reference to a pointer from OpName, etc. A
reproducer is attached as a new test case.
This PR also fixes wrong type inference for IR patterns which generate
new virtual registers without SPIRV type. Previous implementation
assumed always that result has the same address space as a source that
is not the fact, and, for example, led to impossibility to emit a
ptr-cast operation in the reproducer, because wrong type inference
rendered source and destination with the same address space, eliminating
translation of G_ADDRSPACE_CAST.
