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

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added inline comments.


================
Comment at: lib/Target/AMDGPU/AMDGPUInline.cpp:180-182
+  size_t Size = Caller->size() + Callee->size() - 1;
+  if (MaxBB && Size > MaxBB)
+    return InlineCost::getNever();
----------------
rampitec wrote:
> arsenm wrote:
> > This heuristic doesn't make any sense to me. Why does the block count matter? Just use the default cost.
> That is to prevent huge compilation time of some programs. Not an ideal heuristic, but better than nothing.
Compilation time from what? That it requires this custom wrapper function checking sounds like additional motivation to drop it.


================
Comment at: lib/Target/AMDGPU/AMDGPUInline.cpp:176-177
+
+  if (isWrapperOnlyCall(CS))
+    return llvm::InlineCost::getAlways();
+
----------------
By doing this you could possibly be allowing inlining with incompatible function attributes. llvm:::getInlineCost has the additional functionsHaveCompatibleAttributes check, which is more than the above areInlineCompatible


================
Comment at: lib/Target/AMDGPU/AMDGPUTargetTransformInfo.h:165-166
                            const Function *Callee) const;
+
+  unsigned getInliningThresholdMultiplier() { return 9; }
 };
----------------
How did you decide 9 for this?


https://reviews.llvm.org/D36849