[PATCH] D65300: [clang] [CodeGen] clang-misexpect prototype for compiler warnings

[Paul Kirth via Phabricator via cfe-commits]

paulkirth marked 2 inline comments as done.
paulkirth added inline comments.


================
Comment at: clang/lib/CodeGen/MisExpect.cpp:82
+  if (ExpectedTrueBranch) {
+    const llvm::BranchProbability HotFunctionThreshold(1, 100);
+    Scaled = HotFunctionThreshold.scale(CurrProfCount);
----------------
phosek wrote:
> Are these thresholds defined anywhere as constants?
These thresholds come from [[ https://github.com/llvm/llvm-project/blob/7ea131c20c113b78085301a1acd7b28884ec131e/llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp#L1084 |  PGOInstrumentation.cpp:1084 ]]

I will update with a reference to the code that this comes from, but, as noted in the TODO we need a better heuristic. 


================
Comment at: clang/lib/CodeGen/MisExpect.cpp:140-146
+void EmitMisExpectWarning(const CallExpr *Call, CodeGenModule &CGM) {
+  SourceLocation ExprLoc = Call->getBeginLoc();
+  unsigned DiagID = CGM.getDiags().getCustomDiagID(
+      DiagnosticsEngine::Warning, "Current PGO counters disagree with "
+                                  "the use of __builtin_expect().");
+  CGM.getDiags().Report(ExprLoc, DiagID);
+}
----------------
lebedev.ri wrote:
> This is rather undescriptive.
> Can you output some more useful info?
Do you have a suggestion about what feedback would be more useful?

My initial thought with the somewhat generic message was to simply point out that this usage looked problematic, and let the developer investigate. I wasn't sure we wanted to expose details of the internal heuristic to the user by reporting the internal thresholds.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65300