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

[Ruiling, Song via Phabricator via llvm-commits]

ruiling added reviewers: arsenm, nhaehnle.
ruiling added a comment.
Herald added a subscriber: wdng.

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.

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.

Aside from this, I think whether we should treat a `S_AND_XXX exec` 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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121277