[PATCH] D29214: [AMDGPU] Internalize non-kernel symbols

[Tony Tye via Phabricator via llvm-commits]

tony-tye added inline comments.


================
Comment at: lib/Target/AMDGPU/AMDGPUTargetMachine.cpp:88-90
+static cl::opt<bool> WholeProgram(
+  "amdgpu-whole-program",
+  cl::desc("Enable whole program mode"),
----------------
rampitec wrote:
> rampitec wrote:
> > arsenm wrote:
> > > This isn't a good name (and shouldn't it default to true?). How about amdgpu-internalize-functions/
> > It also internalizes global variables, so "functions" here are misleading.
> > That shall not be on by default. For instance this is not suitable to build library. Instead I will submit a patch to add the option from the OpenCL itself (same applies to HCC).
> Do you feel -amdgpu-internalize-symbols is better?
I don't think it should be on by default at all optimization levels as it removes unused global variables and the debugger prefers that all variables remain. So perhaps -O0 (or when -g is specified with the debugger abi, which can be queried from the subtarget by debuggerSupported()) it should not run, or at least the global variable elimination part should not run.

At higher optimization running it by default seems reasonable as it drastically reduces the code object size. But only if the language does not support separate compilation (which is the case currently).


Repository:
  rL LLVM

https://reviews.llvm.org/D29214