[PATCH] D101211: [ADT] Remove StatisticBase and make NoopStatistic empty

Fangrui Song via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sat Apr 24 16:25:10 PDT 2021


MaskRay added a subscriber: kbarton.
MaskRay added inline comments.


================
Comment at: llvm/lib/Transforms/Scalar/LoopFuse.cpp:1540-1545
     ORE.emit(RemarkKind(DEBUG_TYPE, Stat.getName(), FC0.L->getStartLoc(),
                         FC0.Preheader)
              << "[" << FC0.Preheader->getParent()->getName()
              << "]: " << NV("Cand1", StringRef(FC0.Preheader->getName()))
              << " and " << NV("Cand2", StringRef(FC1.Preheader->getName()))
              << ": " << Stat.getDesc());
----------------
MaskRay wrote:
> dblaikie wrote:
> > Losing this remark seems unfortunate - looks like this contradicts the statement in the patch description that these pointers are unused? Mostly unused, but not entirely.
> > 
> > Perhaps this use could be migrated to something else? Or perhaps the Statistic classes could store the strings as char[] members (using some SFINAE/factory template to get the right sizes) so they aren't pointers - if that would address the issue?
> LoopFuse is one of the disabled loop passes (https://lists.llvm.org/pipermail/llvm-dev/2021-April/149737.html).
> We don't know how well it works. We don't know whether using `OptimizationRemarkMissed` will be noisy or not (feasible for optimization remarks). Replacing llvm::Statistic with llvm::TrackingStatistic can make such usage work, but I am not sure it is suitable. So I choose the simplest approach `#if LLVM_ENABLE_STATS`.
@kbarton 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101211



More information about the llvm-commits mailing list