[PATCH] D150447: [MachineSink] Don't reject sinking because of a dead def in isProfitableToSinkTo()

[Jonas Paulsson via Phabricator via llvm-commits]

jonpa added a comment.

In D150447#4361413 <https://reviews.llvm.org/D150447#4361413>, @alexfh wrote:

> As a part of our routine internal release testing we found out that the Ptrdist/yacr2 benchmark <https://github.com/llvm/llvm-test-suite/tree/main/MultiSource/Benchmarks/Ptrdist/yacr2> shows an around 10% regression after this commit on Haswell CPUs. It's not blocking us in any way, but you might be interested in confirming that this doesn't indicate an undesired side effect on the generated code.

It's hard to say exactly what the reason is for that regression, but my best guess would be increased spilling which I think could happen when you increase sinking.  This patch only says that it shouldn't affect the sinking decision if there is a dead def of a phys reg which is not live into the target MBB. That shouldn't affect reg-pressure / spilling by itself. Maybe this increased spilling (if that is indeed the case) comes from other factors which would then call for an additional further refinement of this cost function. Or it could be something else that needs fixing.

I checked impact on SystemZ / SPEC, which looked fine (no big changes). Don't know about yacr2 on Haswell - somebody more familiar would need to look into that...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150447