[PATCH] D121277: [MachineSink] Check block prologue does not clobber uses

[Carl Ritson via Phabricator via llvm-commits]

critson added a comment.

In D121277#3381838 <https://reviews.llvm.org/D121277#3381838>, @ruiling wrote:

> I agree this issue should be fixed here. Machine sinking should check for register dependency between the sunk instruction and the prologue instruction in the successor block.
> But I think there are two kinds of register dependency need to be checked:
>
>   a.) the definition of a register which would be used in successor prologue instruction.
>   b.) the instruction which has a source physical register being overwritten by successor prologue instruction.
>
> I think you are fixing the second kind of dependency currently. Maybe check the first kind of dependency in the same change?
> The first kind of def-use dependency should also apply to the pre-RA sinking.

Presumably (a) only applies to pre-RA sinking? As post-RA everything should be physical registers?
At the moment I think we are getting away without checking because most virtual registers are still unique from SSA form.
However, I agree the risk exists.

> I am not sure whether other uses of SkipPHIsXXX() in the backend would also need such kind of register dependency check, but I think that needs to be investigated separately.

Agreed.

> Aside from this, whether we should treat a `S_AND_XXX exec` as prologue might change in the future. IMO, it is only `S_OR_XXX exec` like instructions which would activating more threads should really be treated as prologue instruction.

Yes, it would be preferable to narrow the definition of prologue instructions in the long run.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121277