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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
