[PATCH] D36849: [AMDGPU] Port of HSAIL inliner

[Alfred Huang via Phabricator via llvm-commits]

alfred.j.huang added a comment.

Look fine.   Cosmetic questions from me.  Thanks!



================
Comment at: lib/Target/AMDGPU/AMDGPUInline.cpp:117
+    uint64_t AllocaSize = 0;
+    for (Value *PtrArg : CS.args()) {
+      if (!PtrArg->getType()->isPointerTy())
----------------
Accumulation of all pointer args to private array, if total size exceeds ArgAllocaCutoff, then bail out.  Look fine.  I was just confused initially with the comment.


================
Comment at: lib/Target/AMDGPU/AMDGPUInline.cpp:186
+  LocalParams.DefaultThreshold = (int)getInlineThreshold(CS);
+  OptimizationRemarkEmitter ORE(CS.getCaller());
+  std::function<AssumptionCache &(Function &)> GetAssumptionCache =
----------------
CS.getCaller() doesn't need to be called again?  Caller set previously.


================
Comment at: lib/Target/AMDGPU/AMDGPUTargetMachine.cpp:329
                      (getTargetTriple().getArch() == Triple::amdgcn);
-  bool EarlyInline = EarlyInlineAll && EnableOpt;
+  bool EarlyInline = EarlyInlineAll && EnableOpt && !EnableAMDGPUFunctionCalls;
   bool AMDGPUAA = EnableAMDGPUAliasAnalysis && EnableOpt;
----------------
This mean if EnableAMDGPUFunctionCalls is set, we will not have early inline anymore?


================
Comment at: test/CodeGen/AMDGPU/amdgpu-inline.ll:43
+; GCN: define amdgpu_kernel void @test_inliner
+; GCN-INL1:   %c1 = tail call coldcc float @foo(
+; GCN-INLDEF: %cmp.i = fcmp ogt float %tmp2, 0.000000e+00
----------------
Is existence of "tail call @foo" considered @foo inlined?   @foo should be inlined, right?


https://reviews.llvm.org/D36849