[PATCH] D32624: Memory intrinsic value profile optimization: Avoid divide by 0

[Teresa Johnson via Phabricator via llvm-commits]

tejohnson added inline comments.


================
Comment at: lib/Transforms/Instrumentation/IndirectCallPromotion.cpp:877
+  // scale up the counts properly (and there is no profitable transformation).
+  if (TotalCount == 0)
+    return false;
----------------
davidxl wrote:
> tejohnson wrote:
> > davidxl wrote:
> > > Or change it to 
> > > 
> > > if (TotalCount < MemOpCountThreshold)
> > >  return false; 
> > > 
> > > and move this check before the actual count check.
> > I could move this up, but note that the counts will be scaled up by ActualCount/SavedTotalCount (where SavedTotalCount == TotalCount at this point). So it is possible for TotalCount < MemOpCountThreshold but not ActualCount, and not the scaled counts. So that check seems overly conservative (and will have an effect other than just preventing 0 divides). 
> The scale is actually to scale down for most the cases -- this is because BB's  count will be updated after inlining, while value profiling total count will be updated. In other words, it won't cause conservative behavior for hot sites.
I checked in one of our large apps, and there are over 44K memory intrinsics where we scale up the counts. So at the least, I would like to consider that change separately from this bugfix.


https://reviews.llvm.org/D32624