[PATCH] D18560: [TTI] Add getInliningThresholdMultiplier.

Matt Arsenault via llvm-commits llvm-commits at lists.llvm.org
Tue Mar 29 09:47:26 PDT 2016


arsenm added inline comments.

================
Comment at: lib/Analysis/TargetTransformInfo.cpp:71-73
@@ +70,5 @@
+    const Function *Caller) const {
+  double Multiplier = TTIImpl->getInliningThresholdMultiplier(Caller);
+  assert(!std::isnan(Multiplier) && "Multiplier cannot be nan");
+  assert(!std::isinf(Multiplier) && "Multiplier cannot be infinity");
+  return Multiplier;
----------------
jlebar wrote:
> arsenm wrote:
> > I thought using host FP was to be avoided, so I'm surprised this isn't an integer factor. Couldn't you have one assert on isnormal()?
> > I thought using host FP was to be avoided
> 
> No idea.  It seems substantially less flexible to use an integer factor (you can't decrease the inlining threshold except to 0), but it works fine for my case, so I have no problem changing it if that's the convention.
> 
> > Couldn't you have one assert on isnormal()?
> 
> isnormal(0) seems to be false, but giving a multiplier of 0 doesn't seem outrageous to me?
You could return a rational numerator + denominator pair


http://reviews.llvm.org/D18560





More information about the llvm-commits mailing list