[PATCH] D101310: [AMDGPU] Replace uses of LDS globals within non-kernel functions by pointers.

[Jon Chesterfield via Phabricator via llvm-commits]

JonChesterfield added a comment.

I can't work out from the implementation, or the comments, what the condition is for deciding whether to transform an LDS variable. Exactly which cases is this intended to transform?

Emitting stores in the entry block is probably not sufficient to ensure the pointers have the right value when later read from. The stores will look dead and can be eliminated or moved across the function calls. One of the attractions of implementing LDS initializers in the back end is ensuring that such transforms cannot break the lowering. Alternatively, the lower module pass + backend coupling could be updated so that an object can be used instead of zero, at which point the IR passes will natively understand the connection.

In order to exercise this code at runtime, we need executable code that uses LDS from functions. I think that will be necessary to be confident that the transform proposed here works correctly. Possibly also a microbenchmark to determine which cases this makes faster as well.



================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUReplaceLDSUseWithPointer.cpp:84
+
+  report_fatal_error(ErrStr);
+}
----------------
This is an optimisation. Instead of a fatal error, it can always leave the variable unchanged. Therefore it should not abort compilation.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101310