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

Sat Apr 24 16:24:18 PDT 2021

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());
----------------
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`.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101211