[PATCH] D94153: [AMDGPU][Inliner] Remove amdgpu-inline and add new TTI inline hooks

Wed Jan 6 10:43:36 PST 2021

arsenm added inline comments.

================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUTargetTransformInfo.cpp:1152
+
+bool GCNTTIImpl::mustNotInline(const CallBase *CB) const {
+  const Function *Caller = CB->getCaller();
----------------
aeubanks wrote:
> rampitec wrote:
> > arsenm wrote:
> > > rampitec wrote:
> > > > aeubanks wrote:
> > > > > arsenm wrote:
> > > > > > aeubanks wrote:
> > > > > > > arsenm wrote:
> > > > > > > > I'm not sure I like having a target hook for a hack like this
> > > > > > > Would you rather just remove this altogether?
> > > > > > I don't remember the story here. @rampitec @vpykhtin @dfukalov ?
> > > > > It was added in https://reviews.llvm.org/D62917.
> > > > > Apparently it's a hack to help with compile times. That's a bit surprising, is this an AMDGPU specific issue?
> > > > It is AMDGPU specific only in a sense. We tend to inline a lot, much more than other targets. Therefor we can have drastic compile time issues. In particular there are several pretty big codes which compile hours instead of one or two minutes without this (suboptimal) cutoff.
> > > Is it possible this was only due to a bug in the subtarget feature compatibility handling? I believe the standard inline analysis specifically looks for wrappers as well
> > It did not work this way at least back when it was added. I think we should not change the logic with this patch. This patch is infrastructural, if we want to tune heuristics that would be a separate work.
> That's fair. I'll keep everything for now.
> 
> @arsenm Any thoughts on alternatives to target hooks?
I think we should first try to remove this part and see how it goes

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D94153/new/

https://reviews.llvm.org/D94153