llvm dev - Oct 2021 - Machine verification and LiveIntervals

Hi,

I've been working on two related codegen cleanups: enabling the
machine verifier at more places in the codegen pass pipeline, and
finishing the transition of TwoAddressInstructionPass from
LiveVariables to LiveIntervals. I'd appreciate help with either or
both of them: code reviews, comments, patches, advice, testing etc.

1. Enabling the machine verifier at more places in the codegen pass pipeline

Currently verification is disabled (by passing false as a second
argument to TargetPassConfig::addPass) in various places, mostly
around register allocation and after addPreEmitPass. I think we should
strive to have it enabled everywhere.

My current work in progress branch is here:
https://github.com/jayfoad/llvm-project/tree/enable-machine-verifier

The outstanding patches are:
https://reviews.llvm.org/D110689 [DetectDeadLanes] Enable machine
verification after this pass
https://reviews.llvm.org/D110695 [ProcessImplicitDefs] Enable machine
verification after this pass
https://reviews.llvm.org/D110697 [UnreachableMachineBlockElim] Enable
machine verification after this pass
https://reviews.llvm.org/D110700 [LiveVariables] Skip verification of
kills inside bundles
https://reviews.llvm.org/D110703 [MachineLoopInfo] Enable machine
verification after this pass
https://reviews.llvm.org/D110829 [X86] Copy registers in reverse order
in convertToThreeAddress
https://reviews.llvm.org/D110834 [PHIElimination] Account for
INLINEASM_BR when inserting kills
https://reviews.llvm.org/D110848 [TwoAddressInstruction] Fix
ReplacedAllUntiedUses in processTiedPairs

The status of that branch, based on running llvm-check-codegen with
LLVM_ENABLE_EXPENSIVE_CHECKS on, is:
- verification is still disabled after addPreEmitPass, but only
because of problems with the Hexagon VLIW packetizer.
- it is still disabled after TwoAddressInstructionPass, but only
because of an AMDGPU subreg problem that I haven't understood yet.
- it is still disabled after PHIElimination because of a problem with
updating LiveVariables that shows up on ARM and Thumb2 tests: "Block
should not be in AliveBlocks" / "Block missing from AliveBlocks"

2. Finishing the transition of TwoAddressInstructionPass from
LiveVariables to LiveIntervals

This is work that was started by Jakob Stoklund Olesen and Cameron
Zwarich back in 2013 (thread at
https://lists.llvm.org/pipermail/llvm-dev/2013-February/059261.html)
but never completed. TwoAddressInstructionPass is almost(*) the only
user of LiveVariables. It already has some support for LiveIntervals.
When that is completed, we can switch to using LiveIntervals
throughout and remove LiveVariables from the codebase. There is a
command line flag -early-live-intervals that you can use to test the
switchover.

Apart from paying off technical debt, the advantage of switching to
LiveIntervals is that it gives you accurate subregister liveness
information. With LiveVariables you have to rely on kill flags, which
apply to the whole "super" register not to individual subregs. (There
may be other advantages but that is the one I'm interested in.)

My current work in progress branch is here:
https://github.com/jayfoad/llvm-project/tree/repair-live-intervals

The outstanding patches are:
https://reviews.llvm.org/D110411 [LiveIntervals] Update subranges in
processTiedPairs
https://reviews.llvm.org/D110182 [LiveIntervals] Find better anchoring
end points when repairing ranges

The status of that branch, based on running the ~18500 tests in
llvm-check-codegen with -early-live-intervals forced on, is:
- 76 tests crash, fail assertions, or fail machine verification
- 215 other tests fail, but //hopefully// only due to minor
differences in the generated code

(*) Actually the AMDGPU target has a pass that uses LiveVariables, but
that was introduced quite recently and should be easy to convert to
LiveIntervals.

Thanks!
Jay.

An update on machine verification: this is now enabled around all
register-allocation-related passes (phi elimination, two address
instruction etc). Thanks Matthias Braun and others for the reviews.
"Let's hope it sticks."

Machine verification is still disabled after calling the target's
addPreEmitPass, because of problems with some late Hexagon and AMDGPU
passes. I have proposed an alternative way of implementing this, so
that verification is only disabled on the targets that actually have
the problems:
https://reviews.llvm.org/D111397 "RFC: A new way of skipping machine
verification after problematic passes"

Thanks,
Jay.

On Fri, 1 Oct 2021 at 10:55, Jay Foad <jay.foad at gmail.com>
wrote:
>
> Hi,
>
> I've been working on two related codegen cleanups: enabling the
> machine verifier at more places in the codegen pass pipeline, and
> finishing the transition of TwoAddressInstructionPass from
> LiveVariables to LiveIntervals. I'd appreciate help with either or
> both of them: code reviews, comments, patches, advice, testing etc.
>
> 1. Enabling the machine verifier at more places in the codegen pass
pipeline
>
> Currently verification is disabled (by passing false as a second
> argument to TargetPassConfig::addPass) in various places, mostly
> around register allocation and after addPreEmitPass. I think we should
> strive to have it enabled everywhere.
>
> My current work in progress branch is here:
> https://github.com/jayfoad/llvm-project/tree/enable-machine-verifier
>
> The outstanding patches are:
> https://reviews.llvm.org/D110689 [DetectDeadLanes] Enable machine
> verification after this pass
> https://reviews.llvm.org/D110695 [ProcessImplicitDefs] Enable machine
> verification after this pass
> https://reviews.llvm.org/D110697 [UnreachableMachineBlockElim] Enable
> machine verification after this pass
> https://reviews.llvm.org/D110700 [LiveVariables] Skip verification of
> kills inside bundles
> https://reviews.llvm.org/D110703 [MachineLoopInfo] Enable machine
> verification after this pass
> https://reviews.llvm.org/D110829 [X86] Copy registers in reverse order
> in convertToThreeAddress
> https://reviews.llvm.org/D110834 [PHIElimination] Account for
> INLINEASM_BR when inserting kills
> https://reviews.llvm.org/D110848 [TwoAddressInstruction] Fix
> ReplacedAllUntiedUses in processTiedPairs
>
> The status of that branch, based on running llvm-check-codegen with
> LLVM_ENABLE_EXPENSIVE_CHECKS on, is:
> - verification is still disabled after addPreEmitPass, but only
> because of problems with the Hexagon VLIW packetizer.
> - it is still disabled after TwoAddressInstructionPass, but only
> because of an AMDGPU subreg problem that I haven't understood yet.
> - it is still disabled after PHIElimination because of a problem with
> updating LiveVariables that shows up on ARM and Thumb2 tests: "Block
> should not be in AliveBlocks" / "Block missing from
AliveBlocks"
>
> 2. Finishing the transition of TwoAddressInstructionPass from
> LiveVariables to LiveIntervals
>
> This is work that was started by Jakob Stoklund Olesen and Cameron
> Zwarich back in 2013 (thread at
> https://lists.llvm.org/pipermail/llvm-dev/2013-February/059261.html)
> but never completed. TwoAddressInstructionPass is almost(*) the only
> user of LiveVariables. It already has some support for LiveIntervals.
> When that is completed, we can switch to using LiveIntervals
> throughout and remove LiveVariables from the codebase. There is a
> command line flag -early-live-intervals that you can use to test the
> switchover.
>
> Apart from paying off technical debt, the advantage of switching to
> LiveIntervals is that it gives you accurate subregister liveness
> information. With LiveVariables you have to rely on kill flags, which
> apply to the whole "super" register not to individual subregs.
(There
> may be other advantages but that is the one I'm interested in.)
>
> My current work in progress branch is here:
> https://github.com/jayfoad/llvm-project/tree/repair-live-intervals
>
> The outstanding patches are:
> https://reviews.llvm.org/D110411 [LiveIntervals] Update subranges in
> processTiedPairs
> https://reviews.llvm.org/D110182 [LiveIntervals] Find better anchoring
> end points when repairing ranges
>
> The status of that branch, based on running the ~18500 tests in
> llvm-check-codegen with -early-live-intervals forced on, is:
> - 76 tests crash, fail assertions, or fail machine verification
> - 215 other tests fail, but //hopefully// only due to minor
> differences in the generated code
>
> (*) Actually the AMDGPU target has a pass that uses LiveVariables, but
> that was introduced quite recently and should be easy to convert to
> LiveIntervals.
>
> Thanks!
> Jay.