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

[Stanislav Mekhanoshin via Phabricator via llvm-commits]

rampitec added inline comments.


================
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;
----------------
alfred.j.huang wrote:
> This mean if EnableAMDGPUFunctionCalls is set, we will not have early inline anymore?
We will not have early inline *all*. We always have selective early inline because we are running opt with the FE before linking, but w/o this new inlining pass it was a pristine llvm simple inliner which did not inline a lot of functions where we would like to see them inlined. Now the default inliner is replaced with our more aggressive version. If still runs twice with both invocations of opt, but it does not need a special pass to mark every function always_inline (which is in a nutshell what -amdgpu-early-inline-all does).

Moreover, w/o this change even if we enable calls we will get no calls, as all of the functions will be marked always_inline and then inlined. Then eventually the whole AMDGPUAlwaysInlinePass pass shall be removed when we enable calls.


================
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
----------------
alfred.j.huang wrote:
> Is existence of "tail call @foo" considered @foo inlined?   @foo should be inlined, right?
It is GCN-INL1 check, which constitutes to the call with -inline-threshold=1. I.e. for the test purposes I'm practically asking not to inline conventional functions, unless they have a really good reason to be inlined (like use of scratch).

The check GCN-INLDEF below represents default threshold under which foo supposed to be inlined.


https://reviews.llvm.org/D36849