If the copyables have parents, used in PHI nodes, this causes complex
schedulable/non-schedulable dependecies, which require complex
processing, but with small profitability. Cut such case early for now to
prevent compiler crashes and compile time blow up.
Fixes#176658
`OwningArrayRef` has several problems.
The naming is strange: `ArrayRef` is specifically a non-owning view, so
the name means "owning non-owning view".
It has a const-correctness bug that is inherent to the interface.
`OwningArrayRef<T>` publicly derives from `MutableArrayRef<T>`. This
means that the following code compiles:
```c++
void const_incorrect(llvm::OwningArrayRef<int> const a) {
a[0] = 5;
}
```
It's surprising for a non-reference type to allow modification of its
elements even when it's declared `const`. However, the problems from
this inheritance (which ultimately stem from the same issue as the weird
name) are even worse. The following function compiles without warning
but corrupts memory when called:
```c++
void memory_corruption(llvm::OwningArrayRef<int> a) {
a.consume_front();
}
```
This happens because `MutableArrayRef::consume_front` modifies the
internal data pointer to advance the referenced array forward. That's
not an issue for `MutableArrayRef` because it's just a view. It is an
issue for `OwningArrayRef` because that pointer is passed as the
argument to `delete[]`, so when it's modified by advancing it forward it
ceases to be valid to `delete[]`. From there, undefined behavior occurs.
It is less convenient than `llvm::SmallVector` for construction. By
combining the `size` and the `capacity` together without going through
`std::allocator` to get memory, it's not possible to fill in data with
the correct value to begin with. Instead, the user must construct an
`OwningArrayRef` of the appropriate size, then fill in the data. This
has one of two consequences:
1. If `T` is a class type, we have to first default construct all of the
elements when we construct `OwningArrayRef` and then in a second pass we
can assign to those elements to give what we want. This wastes time and
for some classes is not possible.
2. If `T` is a built-in type, the data starts out uninitialized. This
easily forgotten step means we access uninitialized memory.
Using `llvm::SmallVector`, by constrast, has well-known constructors
that can fill in the data that we actually want on construction.
`OwningArrayRef` has slightly different performance characteristics than
`llvm::SmallVector`, but the difference is minimal.
The first difference is a theoretical negative for `OwningArrayRef`: by
implementing in terms of `new[]` and `delete[]`, the implementation has
less room to optimize these calls. However, I say this is theoretical
because for clang, at least, the extra freedom of optimization given to
`std::allocator` is not yet taken advantage of (see
https://github.com/llvm/llvm-project/issues/68365)
The second difference is slightly in favor of `OwningArrayRef`:
`sizeof(llvm::SmallVector<T>) == sizeof(void *) * 3` on pretty much any
implementation, whereas `sizeof(OwningArrayRef) == sizeof(void *) * 2`
which seems like a win. However, this is just a misdirection of the
accounting costs: array-new sticks bookkeeping information in the
allocated storage. There are some cases where this is beneficial to
reduce stack usage, but that minor benefit doesn't seem worth the costs.
If we actually need that optimization, we'd be better served by writing
a `DynamicArray` type that implements a full vector-like feature set
(except for operations that change the size of the container) while
allocating through `std::allocator` to avoid the pitfalls outlined
earlier.
If the the node to throttle is a vector node, which is used in split
node, and at least one scalar of such a node is used in many split
nodes, such vector node should be throttled. otherise there might be
wrong def-use chain, which crashes the compiler.
Fixes#175967
Based on Michael Maitland's previous work:
https://github.com/llvm/llvm-project/pull/121222
This PR uses the existing recurrences code instead of introducing a
new pass just for CSA autovec. I've also made recipes that are more
generic.
If the user instruction is used several times in the node, and in one
cases its operand is copyable, but in another is not, need to check all
operands to be sure we do not miss scheduling
If the user instruction is used several times in the node, and in one
cases its operand is copyable, but in another is not, need to check all
operands to be sure we do not miss scheduling
strided-stores-vectorized.ll crashes for RV32 without fixing the
relevant logic in vectorizeTree, because the argument can't be
represented as a 32-bit unsigned value:
```
llvm::APInt::APInt(unsigned int, uint64_t, bool, bool): Assertion `llvm::isUIntN(BitWidth, val) && "Value is not an N-bit unsigned value"' failed.
```
It is intended to be signed, so we simply use ConstantInt::getSigned
instead. This fixes other stride-related instances in the file as well.
For further context, this change is part of unblocking rv32gcv
llvm-test-suite in CI.
The compiler should not generate subvectors with the same extractelement
instructions, it may cause a crash and leads to inefficient
vectorization.
Fixes#174773
If the node is non-scedulable, all instructions are used outside only
and parent is non-schedulable non-phi node, the dependency count should be
increased for such nodes
Fixes#174599
Suppose we are given pointers of the form: `%b + x * %s + y * %c_i`
where `%c_i`s are constants and %s is a run-time fixed value.
If the pointers can be rearranged as follows:
```
%b + 0 * %s + 0
%b + 0 * %s + 1
%b + 0 * %s + 2
...
%b + 0 * %s + w
%b + 1 * %s + 0
%b + 1 * %s + 1
%b + 1 * %s + 2
...
%b + 1 * %s + w
...
```
It means that the memory can be accessed with a strided loads of width `w`
and stride `%s`.
This is motivated by x264 benchmark.
Currently reductions can handles only same/alternate instructions,
skipping potential support for copyables. Patch adds support for
copyables in the reduced values.
Recommit after revert in 1febc3f088ef444af378c0a90aaba2195c30472b
Currently reductions can handles only same/alternate instructions,
skipping potential support for copyables. Patch adds support for
copyables in the reduced values.
If any user of the extractelement instruction is part of the node to be
deleted/gathered, such extractelements instructions should not be
considered for deletion.
Fixes#174020
Initial support for SLP tree throttling. Trims non-profitable subtrees,
trying to maximize perf gains.
Does not support trees with gathered loads yet, since they are not quite
trees, but graphs. Analysis should be added later.
Reviewers: RKSimon
Pull Request: https://github.com/llvm/llvm-project/pull/162018
Recommit after revert in 6ec2ec4826b51d7d809fe08b36883a78d7dc0b98 with
a fix
Initial support for SLP tree throttling. Trims non-profitable subtrees,
trying to maximize perf gains.
Does not support trees with gathered loads yet, since they are not quite
trees, but graphs. Analysis should be added later.
Reviewers: RKSimon
Pull Request: https://github.com/llvm/llvm-project/pull/162018
If the RHS operand is used as a first operand in the bool reduction op,
used as a second operand in the reduction ops, still need to use this
RHS as RHS, not as LHS
https://alive2.llvm.org/ce/z/pmc2YJFixes#173796
If the LHS of the first reduction op is not a first operand, but RHS is,
and RHS is the second operand of the first reductoin op, still need to
emit RHS as a second reduction operand, though without freeze of the
LHS operand
https://alive2.llvm.org/ce/z/2_JLBuFixes#173784
Patch enables support for float point math operations as base
instructions for copyable elements. It also fixes some scheduling
issues, found during testing
Reviewers: hiraditya, RKSimon
Pull Request: https://github.com/llvm/llvm-project/pull/169857
Recommit after reverts in 9008922707915a6632fb74ed301bce11d8775e2a and
c2441689830fcb2588673dedba98da1219a2fb9e.
c2441689830fcb2588673dedba98da1219a2fb9e was caused by other issues, not
related to this patch directly
Patch enables support for float point math operations as base
instructions for copyable elements. It also fixes some scheduling
issues, found during testing
Reviewers: hiraditya, RKSimon
Pull Request: https://github.com/llvm/llvm-project/pull/169857
Recommit after revert in 9008922707915a6632fb74ed301bce11d8775e2a
Patch enables support for float point math operations as base
instructions for copyable elements. It also fixes some scheduling
issues, found during testing
Reviewers: hiraditya, RKSimon
Pull Request: https://github.com/llvm/llvm-project/pull/169857
If the buildvector is going to be vector with threshold cost < 0 and all
buildvector scalars are externally used and remain scalar, such a tree
should not be vectorized, it may lead to a compiler hang because same
scalars remain in the function and will be vectorized once again.
Fixes#172609
Need to check if the extractelement instruction is part of other
buildvector node, before trying to mark it for the deletion, otherwise
the compiler may reuse the deleted instruction.
Fixes#172221
Instructions, used outside the block, must be considered the first
choice for the main instructionsin the copyable nodes, to avoid
use-before-def.
Fixes#171055
Just an attempt to simplify some checks, remove extra calls and reorder
checks to make code simpler and faster
Reviewers: RKSimon, hiraditya
Reviewed By: hiraditya
Pull Request: https://github.com/llvm/llvm-project/pull/170382
Currently, DebugCounters work by creating a unique counter ID during
registration, and then using that ID to look up the counter information
in the global registry.
However, this means that anything working with counters has to always go
through the global instance. This includes the fast path that checks
whether any counters are enabled.
Instead, we can drop the counter IDs, and make the counter variables use
CounterInfo themselves. We can then directly check whether the specific
counter is active without going through the global registry. This is
both faster for the fast-path where all counters are disabled, and also
faster for the case where only one counter is active (as the fast-path
can now still be used for all the disabled counters).
After this change, disabled counters become essentially free at runtime,
and we should be able to enable them in non-assert builds as well.
In #160470, there is a discussion about the possibility to explored a
general approach for handling memory intrinsics.
API changes:
- Remove getMaskedMemoryOpCost, getGatherScatterOpCost,
getExpandCompressMemoryOpCost, getStridedMemoryOpCost from
Analysis/TargetTransformInfo.
- Add getMemIntrinsicInstrCost.
In BasicTTIImpl, map intrinsic IDs to existing target implementation
until the legacy TTI hooks are retired.
- masked_load/store → getMaskedMemoryOpCost
- masked_/vp_gather/scatter → getGatherScatterOpCost
- masked_expandload/compressstore → getExpandCompressMemoryOpCost
- experimental_vp_strided_{load,store} → getStridedMemoryOpCost
TODO: add support for vp_load_ff.
No functional change intended; costs continue to route to the same
target-specific hooks.
Need to check if the non-schedulable phi parent node has unique
operands, if the incoming node has copyables, and the node is
commutative. Otherwise, there might be issues with the correct
calculation of the dependencies.
Fixes#168589
The problem with the many def-use chain problems in SLP vectorizer are
related to the fact that some nodes reuse the same instruction as
insertion point. Insertion point is not the instruction, but the place
between instructions. To set it correctly, better to generate pseudo
instruction immediately after the last instruction, and use it as
insertion point. It resolves the issues in most cases.
Fixes#168512#168576
