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

Matt Arsenault via Phabricator via llvm-commits llvm-commits at lists.llvm.org
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



More information about the llvm-commits mailing list