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

[Stanislav Mekhanoshin via Phabricator via llvm-commits]

rampitec added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUTargetTransformInfo.cpp:1143-1147
+    if (isa<ReturnInst>(*std::next(I->getIterator()))) {
+      LLVM_DEBUG(dbgs() << "    Wrapper only call detected: "
+                        << Callee->getName() << '\n');
+      return true;
+    }
----------------
aeubanks wrote:
> arsenm wrote:
> > I'm not convinced we ever really needed this. I believe the standard inline heuristic will always do this anyway
> There was one case where this forced a coldcc call to be inlined where normally it wouldn't be. Of course, why put coldcc on some random function?
> 
> Will remove.
This was added to unwrap several layers of wrappers we have in device lib. Without this heuristic it was not always properly handled by the standard inliner.


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUTargetTransformInfo.cpp:1152
+
+bool GCNTTIImpl::mustNotInline(const CallBase *CB) const {
+  const Function *Caller = CB->getCaller();
----------------
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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94153