[PATCH] D34867: [ORE] Add diagnostics hotness threshold

[Brian Gesiak via Phabricator via llvm-commits]

modocache added inline comments.


================
Comment at: include/llvm/IR/LLVMContext.h:196-199
+  /// \brief Return the minimum hotness value a diagnostic would need in order
+  /// to be included in optimization diagnostics. If there is no minimum, this
+  /// returns None.
+  Optional<uint64_t> getDiagnosticsHotnessThreshold() const;
----------------
anemet wrote:
> What's the difference between None and 0?
I had originally thought that there should be a distinction between "the threshold has been set to zero" and "the threshold has not been set", but taking another look at this patch and the one to Clang, I realize there isn't a need for it. I'll remove the `Optional<>` here, good call!


================
Comment at: lib/Analysis/OptimizationDiagnosticInfo.cpp:159-165
+  // If a diagnostic has a hotness value, then only emit it if its hotness
+  // meets the threshold.
+  auto Threshold = F->getContext().getDiagnosticsHotnessThreshold();
+  if (OptDiag.getHotness() && Threshold &&
+      *OptDiag.getHotness() < *Threshold) {
+    return;
+  }
----------------
anemet wrote:
> You need to add the same code to MachineOptimizationRemarkEmitter::emit() too.
Oh, good catch, thank you! Do you know if there's an existing test for MachineOptimizationRemarkEmitter?


https://reviews.llvm.org/D34867