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

[Stanislav Mekhanoshin via Phabricator via llvm-commits]

rampitec marked 2 inline comments as done.
rampitec 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"),
----------------
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).


================
Comment at: lib/Target/AMDGPU/AMDGPUTargetMachine.cpp:225-226
+          if (const Function *F = dyn_cast<Function>(&GV)) {
+            if (F->isDeclaration())
+                return true;
+            switch (F->getCallingConv()) {
----------------
arsenm wrote:
> Shouldn't this be return false?
We want to preserve intrinsic declarations, so no, this should return true.


Repository:
  rL LLVM

https://reviews.llvm.org/D29214