[PATCH] D103139: [AMDGPU] Replace non-kernel function uses of LDS globals by pointers.

[Mahesha S via Phabricator via llvm-commits]

hsmhsm added a comment.

In D103139#2782013 <https://reviews.llvm.org/D103139#2782013>, @JonChesterfield wrote:

> 



>> Use different passes for different things, and optimisations are different from lowering.

This is right as per theory. But practicality is different from theory. Those should not be contentious statements in a multipass compiler.

>> Strictly I'm not sure this is an optimisation as it burns cycles to reduce memory, and that's only useful if the saved memory improves occupancy.

Run time cost v/s memory cost are endless debatable topic, which cost saving, we should aim for, depends on context and enviroment. Here memory cost is too important one and cannot be neglected, though run time cost is also important one. Further, until we test the system with some heavily LDS bound application, you cannot come to a conlusion that it is useless transformation.

>> I see zero advantages to your single-combined-pass preference and have enumerated disadvantages. Mostly that making it harder to review and to test makes it less likely to work. It also means the tests are fragile, which will cost us when improving the heuristics on which variables to transform.

Yes, these are the dis-advantages of having pointer-replacement code within ModuleLDSLowering pass

>> What are the advantages you see that merit breaking from the default of using multiple passes in a multiple pass compiler?

Here are advantages:

First, this tranformation is not something unique and independent, it has meaning only in the presence of ModuleLDSLowering pass, otherwise it is totally meaningless to have this pointer-replacement transormation pass. Imagine a scenario, where we run pointer-replacement transormation pass, but ModuleLDSLowering pass is disabled.

Second, if there exist some other transormation passes between pointer-replacement pass and ModuleLDSLowering pass which have some side effect on the the pointer-replacement transformed code, then it would cause more trouble to whole intention behind the pass.

Third, logically both are tied together to achieve the common purpose. So it make sense that both exist together in a single pass. OpenMP use-cases may not care about it, but, AMDGPU backend does not only support OpenMP, but also support other GPU languages.

In anycase, I am fine with a separate pass, and I am going to implement it. Stay tuned for a new patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103139