This patch replaces SmallSet<T *, N> with SmallPtrSet<T *, N>. Note
that SmallSet.h "redirects" SmallSet to SmallPtrSet for pointer
element types:
template <typename PointeeType, unsigned N>
class SmallSet<PointeeType*, N> : public SmallPtrSet<PointeeType*, N>
{};
We only have 140 instances that rely on this "redirection", with the
vast majority of them under llvm/. Since relying on the redirection
doesn't improve readability, this patch replaces SmallSet with
SmallPtrSet for pointer element types.
This is the follow up I mentioned doing in the review of 52b345d. That
change introduced an API for performing instruction simplifications
following copy propagation (e.g. things like recognizing ORI a0, a1,
zero is just a move). As noted in that review, we should be able to
perform iterative simplification as we move forward through the block,
but weren't because of the code structure.
The majority of this code is just deleting the special casing for
constant source and destination tracking, and merging the copy handling
with the main path. By assumption, the properties of copies (in terms of
register reads and writes), must be a subset of general instructions.
Once we do that, the iterative bit basically falls out from having the
tracking performed for copies which are recognized *after* we forward
prior uses.
If there's an implicit-def of a super register, the propagation
must preserve this implicit-def. Knowing how and when to do this
may require target specific knowledge so just disable it for now.
Prior to 2def1c4, we checked that the copy had explicit 2 operands
when that was removed we started allowing implicit operands through.
This patch adds a check for implicit operands, but still allows
extra explicit operands which was the goal of 2def1c4.
Fixes#131478.
PR #136875 was posted as a draft PR that handled a subset of these
cases, using the CompressPat mechanism. The consensus from that
discussion (and a conclusion I agree with) is that it would be
beneficial doing this optimisation earlier on, and in a way that isn't
limited just to cases that can be handled by instruction compression.
The most common source for instructions that can be
optimized/canonicalized in this way is through tail duplication in
MachineBlockPlacement followed by machine copy propagation. For RISC-V,
choosing a more canonical instruction allows it to be compressed when it
couldn't be before. There is the potential that it would make other
MI-level optimisations easier.
This modifies ~910 instructions across an llvm-test-suite build
including SPEC2017, targeting rva22u64. Looking at the diff, it seems
there's room for eliminating instructions or further propagating after
this.
Coverage of instructions is based on observations from a script written
to find redundant or improperly canonicalized instructions (though I aim
to support all instructions in a 'group' at once, e.g. MUL* even if I
only saw some variants of MUL in practice).
DenseSet, SmallPtrSet, SmallSet, SetVector, and StringSet recently
gained C++23-style insert_range. This patch replaces:
Dest.insert(Src.begin(), Src.end());
with:
Dest.insert_range(Src);
This patch does not touch custom begin like succ_begin for now.
This change removes 189 static instances of no-op reg-reg moves (i.e.
where src == dest) across llvm-test-suite when compiled for RISC-V
rv64gc and with SPEC included.
llvm/llvm-project@9e436c2daa tries to handle register masks and
sub-registers, it avoids clobbering RegUnit presreved by regmask. But it
then introduces invalid pointer issues.
We delete the copies without invalidate all the use in the CopyInfo, so
we dereferenced invalid pointers in next interation, causing asserts.
Fixes: #126107
---------
Co-authored-by: Matt Arsenault <arsenm2@gmail.com>
When passing an instruction with a register mask, the machine copy
propagation pass was dropping the information about some copy
instructions which define a register which is preserved by the mask,
because that register overlaps a register which is partially clobbered
by it. This resulted in a miscompilation for AArch64, because this
caused a live copy to be considered dead.
The fix is to clobber register masks by finding the set of reg units
which is preserved by the mask, and clobbering all units not in that
set.
This is based on #122472, and fixes the compile time performance
regressions which were caused by that.
When passing an instruction with a register mask, the machine copy
propagation pass was dropping the information about some copy
instructions which define a register which is preserved by the mask,
because that register overlaps a register which is partially clobbered
by it. This resulted in a miscompilation for AArch64, because this
caused a live copy to be considered dead.
The fix is to clobber register masks by finding the set of reg units
which is preserved by the mask, and clobbering all units not in that
set.
Before this patch, redundant COPY couldn't be removed for the following
case:
```
$R0 = OP ...
... // Read of %R0
$R1 = COPY killed $R0
```
This patch adds support for tracking the users of the source register
during backward propagation, so that we can remove the redundant COPY in
the above case and optimize it to:
```
$R1 = OP ...
... // Replace all uses of %R0 with $R1
```
Before this patch, redundant COPY couldn't be removed for the following
case:
```
%reg1 = COPY %const-reg
... // There is a def of %const-reg
%reg2 = COPY killed %reg1
```
where this can be optimized to:
```
... // There is a def of %const-reg
%reg2 = COPY %const-reg
```
This patch allows for such optimization by not invalidating defined
constant registers. This is safe, as architectures like AArch64 and
RISCV replace a dead definition of a GPR with a zero constant register
for certain instructions.
Once we modernize CopyInfo with default member initializations,
Copies.insert({Unit, ...})
becomes equivalent to:
Copies.try_emplace(Unit)
which we can simplify further down to Copies[Unit].
Tail duplication will generate the redundant move before return. It is
because the MachineCopyPropogation can't recognize COPY after post-RA
pseudoExpand.
This patch make MachineCopyPropogation recognize `%0 = ADDI %1, 0` as
COPY
Fixes#82659
There are some functions, such as `findRegisterDefOperandIdx` and `findRegisterDefOperand`, that have too many default parameters. As a result, we have encountered some issues due to the lack of TRI parameters, as shown in issue #82411.
Following @RKSimon 's suggestion, this patch refactors 9 functions, including `{reads, kills, defines, modifies}Register`, `registerDefIsDead`, and `findRegister{UseOperandIdx, UseOperand, DefOperandIdx, DefOperand}`, adjusting the order of the TRI parameter and making it required. In addition, all the places that call these functions have also been updated correctly to ensure no additional impact.
After this, the caller of these functions should explicitly know whether to pass the `TargetRegisterInfo` or just a `nullptr`.
Previously we wouldn't remove dead copies from basic blocks with
successors. The comment said we didn't want to trust the live-in lists.
The comment is very old so I'm not sure if that's still a concern today.
This patch checks the live-in lists and removes copies from
MaybeDeadCopies if they are referenced by any live-ins in any
successors. We only do this if the tracksLiveness property is set. If
that property is not set, we retain the old behavior.
This patch makes a SmallVector slightly larger. We encounter quite a
few instructions with 3 or 4 defs but very few beyond that on X86.
This saves 0.39% of heap allocations during the compilation of a large
preprocessed file, namely X86ISelLowering.cpp, for the X86 target.
Machine Copy Propagation Pass may lose some opportunities to further
remove the redundant copy instructions during the ForwardCopyPropagateBlock
procedure. When we Clobber a "Def" register, we also need to remove the record
from the copy maps that indicates "Src" defined "Def" to ensure the correct semantics
of the ClobberRegister function. This patch reapplies #70778 and addresses the corner
case bug #73512 specific to the AMDGPU backend. Additionally, it refines the criteria
for removing empty records from the copy maps, thereby enhancing overall safety.
For more information, please see the C++ test case generated code in
"vector.body" after the MCP Pass: https://gcc.godbolt.org/z/nK4oMaWv5.
Machine Copy Propagation Pass may lose some opportunities to further
remove the redundant copy instructions during the ForwardCopyPropagateBlock
procedure. When we Clobber a "Def" register, we also need to remove the record
from the copy maps that indicates "Src" defined "Def" to ensure the correct semantics
of the ClobberRegister function.
For more information, please see the C++ test case generated code in
"vector.body" after the MCP Pass: https://gcc.godbolt.org/z/nK4oMaWv5.
We must also track the super sources of a copy, otherwise we introduce a sort of subtle bug.
Consider:
1. DEF r0:r1
2. USE r1
3. r6:r9 = COPY r10:r13
4. r14:15 = COPY r0:r1
5. USE r6
6.. r1:4 = COPY r6:9
BackwardCopyPropagateBlock processes the instructions from bottom up. After processing 6., we will have propagatable copy for r1-r4 and r6-r9. After 5., we invalidate and erase the propagatble copy for r1-r4 and r6 but not for r7-r9.
The issue is that when processing 3., data structures still say we have valid copies for dest regs r7-r9 (from 6.). The corresponding defs for these registers in 6. are r1:r4, which we mark as registers to invalidate. When invalidating, we find the copy that corresponds to r1 is 4. (this was added when processing 4.), and we say that r1 now maps to unpropagatable copies. Thus, when we process 2., we do not have a valid copy, but when we process 1. we do -- because the mapped copy for subregister r0 was never invalidated.
The net result is to propagate the copy from 4. to 1., and replace DEF r0:r1 with DEF r14:r15. Then, we have a use before def in 2.
The main issue is that we have an inconsitent state between which def regs and which src regs are valid. When processing 5., we mark all the defs in 6. as invalid, but only the subreg use as invalid. Either we must only invalidate the individual subreg for both uses and defs, or the super register for both.
Differential Revision: https://reviews.llvm.org//D157564
Change-Id: I99d5e0b1a0d735e8ea3bd7d137b6464690aa9486
Revert D152502 and instead optimize away copy from undefs, but clear the undef flag on the original copy.
Apparently, not optimizing the COPY can cause performance issues in some cases.
Fixes SWDEV-405813, SWDEV-405899
Reviewed By: arsenm
Differential Revision: https://reviews.llvm.org/D153838
I don't think we can safely remove the second COPY as redundant in such cases.
The first COPY (which has undef src) may be lowered to a KILL instruction instead, resulting in no COPY being emitted at all.
Testcase is X86 so it's in the same place as other testcases for this function, but this was initially spotted on AMDGPU with the following:
```
renamable $vgpr24 = PRED_COPY undef renamable $vgpr25, implicit $exec
renamable $vgpr24 = PRED_COPY killed renamable $vgpr25, implicit $exec
```
The second COPY waas removed as redundant, and the first one was lowered to a KILL (= removed too), causing $vgpr24 to not have $vgpr25's value.
Fixes SWDEV-401507
Reviewed By: arsenm
Differential Revision: https://reviews.llvm.org/D152502
This change initializes the members TSI, LI, DT, PSI, and ORE pointer feilds of the SelectOptimize class to nullptr.
Reviewed By: LuoYuanke
Differential Revision: https://reviews.llvm.org/D148303
In this example:
```
$d14 = COPY killed $d18
$s0 = MI $s28
```
$s28 is a sub-register of $d14. However, $d18 does not have
sub-registers and thus cannot be forwarded. Previously, this resulted
in $noreg being substituted in place of the use of $s28, which later
led to an assertion failure.
Fixes https://github.com/llvm/llvm-project/issues/60908, a regression
that was introduced in D141747.
Reviewed By: arsenm
Differential Revision: https://reviews.llvm.org/D146930
This patch mechanically replaces None with std::nullopt where the
compiler would warn if None were deprecated. The intent is to reduce
the amount of manual work required in migrating from Optional to
std::optional.
This is part of an effort to migrate from llvm::Optional to
std::optional:
https://discourse.llvm.org/t/deprecating-llvm-optional-x-hasvalue-getvalue-getvalueor/63716
D125335 makes regsOverlap skip following control flow, which is not entended
in the original code.
Differential Revision: https://reviews.llvm.org/D128039
treated as Copy instruction in MCP.
This is then used in AArch64 to remove copy instructions after taildup
ran in machine block placement
Differential Revision: https://reviews.llvm.org/D125335
Change the implementation of isForwardableRegClassCopy so that it
does not rely on getMinimalPhysRegClass. Instead, iterate over all
classes looking for any that satisfy a required property.
NFCI on current upstream targets, but this copes better with
downstream AMDGPU changes where some new smaller classes have been
introduced, which was breaking regclass equality tests in the old
code like:
if (UseDstRC != CrossCopyRC && CopyDstRC == CrossCopyRC)
Differential Revision: https://reviews.llvm.org/D121903
This reverts commit 7f230feeeac8a67b335f52bd2e900a05c6098f20.
Breaks CodeGenCUDA/link-device-bitcode.cu in check-clang,
and many LLVM tests, see comments on https://reviews.llvm.org/D121169
