[PATCH] D124631: [TwoAddressInstructionPass] Special processing of STATEPOINT instruction.

Thu Apr 28 12:21:31 PDT 2022

dantrushin added a comment.

In D124631#3480820 <https://reviews.llvm.org/D124631#3480820>, @MatzeB wrote:

> Sorry I am not up-to-speed with how STATEPOINTs work in detail. But this change is confusing me and probably needs the description reworked:
>
> - The summary reads like somehow the lifetime for any `STATEPOINT` operand must end at the `STATEPOINT`.
> - The comment above `TwoAddressInstructionPass::processStatepoint` shows the exact opposite and lengthens the lifetime of the operand register.

STATEPOINT is a special pseudo instruction which wraps call and does not exists in hardware. Its only purpose is to represent possible register value change during a call it wraps. (Which we cannot express by other means)
It uses tied registers in conventional way - use and def must be allocated to the same physical register.
Its important property is that at SSA level, USE register is never used after STATEPOINT, only corresponding DEF is. So, as I tried to explain in description, they are adjacent segments of the same Live Range.
This allows us to not insert COPY instruction before STATEPOINT, but simply replace DEF register with USE one. This is what RegisterCoalescer will do anyway, but using quadratic algorithm (extremely expensive).

> Also given that this is a targetted change to the twoaddressinstruction pass it would be good to have at least 1 test that just checks for this particular change in isolation! (the `statepoint-invoke-ra.mir` is running multiple passes in succession and doesn't make it clear what exactly changed).

In theory, this should have been NFC patch, but in practice it affects how Register Allocator works (because there are no `SlotIndexes` holes anymore and LiveRanges are shorter their weights change.

The only purpose of this patch is to eliminate COPY instruction which TwoAddress pass insert before instructions which it does not optimize. I'll add simple test if it would be useful.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D124631/new/

https://reviews.llvm.org/D124631