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

Wed May 26 06:31:06 PDT 2021

hsmhsm added a comment.

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

> I see that. However, we are working on a multipass compiler with a clearly delimited format used between passes.
>
> If your optimisation is written standalone, it can be tested directly. The output doesn't have to embed the lowering pass semantics. It is also easier to write as a separate pass. You have the utils header for sharing code between the two.
>
> I agreed to Stas making the minor adjustments to generalise the lowering pass, for use lowering a function as well as a module. I definitely did not, and do not, agree to splicing an optimisation through the lowering code.
>
> I'm also expecting the optimisation in question to pessimise openmp code, so want to keep an option to disable it from the pipeline when building openmp applications.

Jon,

I get your points. We discussed it in the past and on Friday's meeting. There are advantages and dis-advantages in both the approaches.  I am personally fine with either approach, but what I want is a common agreement on the decision taken and being stick to it (in general on all technical topics).  When I raised this question in Friday meeting, Stas had an openion of having the pointer-repalcement code within this patch itself, and I seconded it. And we asked about your decision, and also said, you are fine too. But, now you are opposing it after the patch is ready. Then how can I trust on you sticking to a decision taken?

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103139