[PATCH] D91516: [AMDGPU] Support for device scope shared variables

Wed Dec 23 19:05:48 PST 2020

hsmhsm added a comment.

In D91516#2470536 <https://reviews.llvm.org/D91516#2470536>, @JonChesterfield wrote:

> I believe the intended effect of this pass is:
>
> - Allow functions called directly to access LDS without being inlined

Yes, this is correct.

> - Silently induce UB in programs containing recursive functions
> - Silently induce UB in some programs containing indirect calls, probably those with complicated data flow

No this assumption is not correct. We never committed to induce "Undefiend Behaviour" of any sort in the program as a part of this feature. All we intended here was to support local function LDS as general as possible (as a first class support), by taking advantage of existing support within LLVM infrastructure.

> The benefit is that some applications which meet those constraints perform better than when the functions are inlined instead, perhaps due to code size effects.

Inlining is not affected anyway by this feature.  If some other pass after this pass decide to inline function within kernel (because of performance advantage etc) it can do it by all means. This pass does not put any constraint on subsequent passes in any way for any thing.

> If that is an accurate characterisation, I don't think it is an improvement on the status quo. Perhaps one to keep in the internal repo?

I do not understand what you mean by above sentence.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91516