[PATCH] D100481: [AMDGPU] Disable forceful inline of non-kernel functions which use LDS.

[Tony Tye via Phabricator via llvm-commits]

t-tye added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUAlwaysInlinePass.cpp:125
+
+  if (!AMDGPUTargetMachine::EnableLowerModuleLDS) {
+    for (GlobalVariable &GV : M.globals()) {
----------------
JonChesterfield wrote:
> hsmhsm wrote:
> > JonChesterfield wrote:
> > > What's REGION_ADDRESS? LowerModuleLDS doesn't to do anything with it so this looks like an incidental behaviour change
> > Just like LOCAL_ADDRESS is specific to work-group,  REGION_ADDRESS is specific to device, but it is not yet implemented in AMDHSA.  Please have a look at the link - https://llvm.org/docs/AMDGPUUsage.html#address-spaces.
> > 
> > DS instructions can be used to access region memory too in addition to local memory. And, I guess, this is the region that code for local memory and region memory co-exist together at some places.
> > 
> > @arsenm might give better answer here about if it will result in an incidental behaviour change.
> Let's refactor this so that it doesn't change behaviour for REGION_ADDRESS, as otherwise we're introducing at least one of confusion or a latent bug.
REGION maps to the AMD GPU GDS (Global Data Store) scratchpad memory hardware. It is basically like LDS (Local Data Store) and uses the same ds instructions, but is shared by all waves on the same device.

The ROCm runtime has not implemented it. An earlier LLVM fork did have some support I believe. There are numerous places in AMD GPU LLVM that do have support for GDS such as in the SIMemoryLegalizer. It is described at https://llvm.org/docs/AMDGPUUsage.html#address-spaces .


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100481