If we have a rematerializable instruction without live virtual register uses (that we didn't heuristically decide to shrink), but with a physreg use, we were creating a kill to keep the physreg operand liveness unchanged. This meant we *weren't* keeping around the remat instruction, and thus inhibiting future remat within the original live interval. If we keep the remat instruction, that *also* keeps the physreg use, and thus we can achieve both objectives at once. Noticed this via inspection, and we don't currently seem to have any rematerializable instructions which could observe the difference. This could in theory happen for both trivial and non-trivial remat, but requires the rematerialization of an instruction with a physreg use.
This is an attempt to simplify the rematerialization logic in InlineSpiller and SplitKit. I'd earlier done the same for RegisterCoalescer in 57b673. The basic idea of this change is that we don't need to check whether an instruction is rematerializable early. Instead, we can defer the check to the point where we're actually trying to materialize something. We also don't need to indirect that query through a VNI key, and can instead just check the instruction directly at the use site.
Reverts llvm/llvm-project#160765. Failures on buildbot indicate second assertion does not in fact hold.
We should always be able to find the VNInfo in the original live interval which corresponds to the subset we're trying to spill, and the only cases where we have a VNInfo without a definition instruction are if the vni is unused, or corresponds to a phi. Adjust the code structure to explicitly check for PHIDef, and assert the stronger conditions.
This aims to fix the issue that caused https://reviews.llvm.org/D106408 to be reverted. CalcSpillWeights will reduce the weight of an interval by half if it's considered rematerializable, so it will be evicted before others. It does this by checking TII.isTriviallyReMaterializable. However rematerialization may still fail if any of the defining MI's uses aren't available at the locations it needs to be rematerialized. LiveRangeEdit::canRematerializeAt calls allUsesAvailableAt to check this but CalcSpillWeights doesn't, so the two diverge. This fixes it by also checking allUsesAvailableAt in CalcSpillWeights. In practice this has zero change AArch64/X86-64/RISC-V as measured on llvm-test-suite, but prevents weights from being perturbed in an upcoming patch which enables more rematerialization by re-attempting https://reviews.llvm.org/D106408
This change builds on https://github.com/llvm/llvm-project/pull/160319 which tries to clarify which *callers* (not backends) assume that the result is actually trivial. This change itself should be NFC. Essentially, I'm just renaming the existing isTrivialRematerializable to the non-trivial version and then adding a new trivial version (with the same name as the prior function) and simplifying a few callers which want that semantic. This change does *not* enable non-trivial remat any more broadly than was already done for our targets which were lying through the old APIs; that will come separately. The goal here is simply to make the code easier to follow in terms of what assumptions are being made where. --------- Co-authored-by: Luke Lau <luke_lau@icloud.com>
LiveRangeEdit's rematerialization checking logic is used in two quite different ways. For SplitKit and InlineSpiller, we're analyzing all defs associated with a live interval, doing that analysis up front, and then using the result a bit later. The RegisterCoalescer, we're analysing exactly one ValNo at a time, and using the legality result immediately. LRE had a checkRematerializable which existed basically to adapt the later into the former usage model. Instead, this change bypasses the ScannedRemat and Remattable structures, and directly queries the underlying routines. This is easy to read, and makes it more clear as to which uses actually need the deferred analysis. (A following change may try to unwind that too, but it's not strictly NFC.)
When rematerialize a virtual register the register may be early clobbered. However rematerializeAt(...) just return the slot index of Slot_Register which cause mis-calculating live range for the register --------- Co-authored-by: Yuanke Luo <ykluo@birentech.com>
Only one caller cares about the true case of this parameter, so move the check to that single caller. Note that RegisterCoalescer seems like it should care, but it already duplicates the check several lines above.