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

[Stanislav Mekhanoshin via Phabricator via llvm-commits]

rampitec added inline comments.


================
Comment at: lib/Target/AMDGPU/AMDGPUInline.cpp:176-177
+
+  if (isWrapperOnlyCall(CS))
+    return llvm::InlineCost::getAlways();
+
----------------
rampitec wrote:
> arsenm wrote:
> > Wrapper calls will (ignoring no inline) always be inlined by the stock inline heuristics, so †his shouldn't be necessary
> That is unless we hit MaxBB, in which case we still want to inline it.
AFAIR I've seen cases when our library call wrappers were not inlined even with relatively small programs.


================
Comment at: lib/Target/AMDGPU/AMDGPUInline.cpp:180-182
+  size_t Size = Caller->size() + Callee->size() - 1;
+  if (MaxBB && Size > MaxBB)
+    return InlineCost::getNever();
----------------
arsenm wrote:
> rampitec wrote:
> > arsenm wrote:
> > > rampitec wrote:
> > > > arsenm wrote:
> > > > > 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.
> > > > The actual testcase which required that was VRay AFAIR. When we increase inline threshold (or inline everything like now) we a vulnerable to extremely high compilation times for huge codes. In case of VRay it was hours, and this has decreased it to minutes.
> > > I meant where was the compile time spent? I doubt it was the inliner itself
> > As usual, optimizing inflated code, scheduling and RA.
> I'd rather drop this workaround for now at least. 
OK


https://reviews.llvm.org/D36849