[llvm-branch-commits] [clang][misexpect] Add support to clang for profitable annotation diagnostics (PR #96525)
via llvm-branch-commits
llvm-branch-commits at lists.llvm.org
Mon Jun 24 10:48:08 PDT 2024
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-llvm-transforms
@llvm/pr-subscribers-clang-driver
Author: Paul Kirth (ilovepi)
<details>
<summary>Changes</summary>
Add basic plumbing to clang so that diagnostics can be surfaced to
users.
---
Full diff: https://github.com/llvm/llvm-project/pull/96525.diff
9 Files Affected:
- (modified) clang/include/clang/Basic/CodeGenOptions.def (+1)
- (modified) clang/include/clang/Driver/Options.td (+4)
- (modified) clang/lib/CodeGen/BackendUtil.cpp (+1)
- (modified) clang/lib/CodeGen/CodeGenAction.cpp (+4)
- (modified) clang/lib/Driver/ToolChains/Clang.cpp (+7)
- (modified) clang/lib/Frontend/CompilerInvocation.cpp (+3)
- (added) clang/test/Profile/Inputs/missing-annotation.proftext (+18)
- (added) clang/test/Profile/missing-annotation.c (+35)
- (modified) llvm/lib/Transforms/Utils/MisExpect.cpp (+4-4)
``````````diff
diff --git a/clang/include/clang/Basic/CodeGenOptions.def b/clang/include/clang/Basic/CodeGenOptions.def
index e3f6da4a84f69..fab91cd8a76b5 100644
--- a/clang/include/clang/Basic/CodeGenOptions.def
+++ b/clang/include/clang/Basic/CodeGenOptions.def
@@ -181,6 +181,7 @@ CODEGENOPT(FatalWarnings , 1, 0) ///< Set when -Wa,--fatal-warnings is
CODEGENOPT(NoWarn , 1, 0) ///< Set when -Wa,--no-warn is enabled.
CODEGENOPT(NoTypeCheck , 1, 0) ///< Set when -Wa,--no-type-check is enabled.
CODEGENOPT(MisExpect , 1, 0) ///< Set when -Wmisexpect is enabled
+CODEGENOPT(MissingAnnotations, 1, 0) ///< Set when suggesting missing perf annotations
CODEGENOPT(EnableSegmentedStacks , 1, 0) ///< Set when -fsplit-stack is enabled.
CODEGENOPT(StackClashProtector, 1, 0) ///< Set when -fstack-clash-protection is enabled.
CODEGENOPT(NoImplicitFloat , 1, 0) ///< Set when -mno-implicit-float is enabled.
diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td
index c529cc9506667..6dfc5bb437034 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -2079,6 +2079,10 @@ def fdiagnostics_misexpect_tolerance_EQ : Joined<["-"], "fdiagnostics-misexpect-
Group<f_Group>, Visibility<[ClangOption, CC1Option]>,
MetaVarName<"<value>">,
HelpText<"Prevent misexpect diagnostics from being output if the profile counts are within N% of the expected. ">;
+defm diagnostics_missing_annotations : BoolFOption<"diagnostics-missing-annotations",
+ CodeGenOpts<"MissingAnnotations">, DefaultFalse,
+ PosFlag<SetTrue,[], [ClangOption, CC1Option], "Enable diagnostics for missing annotations based on profile counts">,
+ NegFlag<SetFalse>>;
defm diagnostics_show_option : BoolFOption<"diagnostics-show-option",
DiagnosticOpts<"ShowOptionNames">, DefaultTrue,
NegFlag<SetFalse, [], [ClangOption, CC1Option]>,
diff --git a/clang/lib/CodeGen/BackendUtil.cpp b/clang/lib/CodeGen/BackendUtil.cpp
index b09680086248d..2bcc23a6a9655 100644
--- a/clang/lib/CodeGen/BackendUtil.cpp
+++ b/clang/lib/CodeGen/BackendUtil.cpp
@@ -486,6 +486,7 @@ static bool initTargetOptions(DiagnosticsEngine &Diags,
Options.MCOptions.PPCUseFullRegisterNames =
CodeGenOpts.PPCUseFullRegisterNames;
Options.MisExpect = CodeGenOpts.MisExpect;
+ Options.MissingAnnotations = CodeGenOpts.MissingAnnotations;
return true;
}
diff --git a/clang/lib/CodeGen/CodeGenAction.cpp b/clang/lib/CodeGen/CodeGenAction.cpp
index 6d3efdb5ffe34..efebe28e5ebc4 100644
--- a/clang/lib/CodeGen/CodeGenAction.cpp
+++ b/clang/lib/CodeGen/CodeGenAction.cpp
@@ -326,6 +326,10 @@ void BackendConsumer::HandleTranslationUnit(ASTContext &C) {
CodeGenOpts.DiagnosticsMisExpectTolerance);
}
+ if (CodeGenOpts.MissingAnnotations) {
+ Ctx.setAnnotationDiagsRequested(true);
+ }
+
// Link each LinkModule into our module.
if (!CodeGenOpts.LinkBitcodePostopt && LinkInModules(getModule()))
return;
diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp
index 2ce9e2f4bcfcd..9704779e00258 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -4243,6 +4243,13 @@ static void RenderDiagnosticsOptions(const Driver &D, const ArgList &Args,
CmdArgs.push_back(Args.MakeArgString(Opt));
}
+ if (const Arg *A =
+ Args.getLastArg(options::OPT_fdiagnostics_missing_annotations,
+ options::OPT_fno_diagnostics_missing_annotations)) {
+ if (A->getOption().matches(options::OPT_fdiagnostics_missing_annotations))
+ CmdArgs.push_back("-fdiagnostics-missing-annotations");
+ }
+
if (const Arg *A = Args.getLastArg(options::OPT_fdiagnostics_format_EQ)) {
CmdArgs.push_back("-fdiagnostics-format");
CmdArgs.push_back(A->getValue());
diff --git a/clang/lib/Frontend/CompilerInvocation.cpp b/clang/lib/Frontend/CompilerInvocation.cpp
index a6d9f42ace9cc..f9d34cd011f2a 100644
--- a/clang/lib/Frontend/CompilerInvocation.cpp
+++ b/clang/lib/Frontend/CompilerInvocation.cpp
@@ -4775,6 +4775,9 @@ bool CompilerInvocation::CreateFromArgsImpl(
}
}
+ if(Args.hasArg(OPT_fdiagnostics_missing_annotations))
+ Res.getCodeGenOpts().MissingAnnotations = true;
+
if (LangOpts.CUDA) {
// During CUDA device-side compilation, the aux triple is the
// triple used for host compilation.
diff --git a/clang/test/Profile/Inputs/missing-annotation.proftext b/clang/test/Profile/Inputs/missing-annotation.proftext
new file mode 100644
index 0000000000000..0bf7158702e28
--- /dev/null
+++ b/clang/test/Profile/Inputs/missing-annotation.proftext
@@ -0,0 +1,18 @@
+bar
+# Func Hash:
+11262309464
+# Num Counters:
+2
+# Counter Values:
+200000
+1
+
+fizz
+# Func Hash:
+11262309464
+# Num Counters:
+2
+# Counter Values:
+200000
+1
+
diff --git a/clang/test/Profile/missing-annotation.c b/clang/test/Profile/missing-annotation.c
new file mode 100644
index 0000000000000..5cf2a87a94c8d
--- /dev/null
+++ b/clang/test/Profile/missing-annotation.c
@@ -0,0 +1,35 @@
+/// Test that missing annotation diagnostics are suggested for hot branches
+
+// note test diagnostics are issued when profiling data mis-matches annotations
+// RUN: llvm-profdata merge %S/Inputs/missing-annotation.proftext -o %t.profdata
+// RUN: %clang %s -O2 -c -S -emit-llvm -o - -fprofile-instr-use=%t.profdata -Xclang -verify=exact -fdiagnostics-missing-annotations -debug-info-kind=line-tables-only -Rpass=missing-annotations
+
+// foo-no-diagnostics
+
+int foo(int);
+int baz(int);
+int buzz();
+
+const int inner_loop = 100;
+const int outer_loop = 2000;
+int bar() {
+ int rando = buzz();
+ int x = 0;
+ if (rando % (outer_loop * inner_loop) == 0) { // exact-remark {{Extremely hot condition. Consider adding llvm.expect intrinsic}}
+ x = baz(rando);
+ } else {
+ x = foo(50);
+ }
+ return x;
+}
+
+int fizz() {
+ int rando = buzz();
+ int x = 0;
+ if (rando % (outer_loop * inner_loop) == 0) { // exact-remark {{Extremely hot condition. Consider adding llvm.expect intrinsic}}
+ x = baz(rando);
+ } else {
+ x = foo(50);
+ }
+ return x;
+}
diff --git a/llvm/lib/Transforms/Utils/MisExpect.cpp b/llvm/lib/Transforms/Utils/MisExpect.cpp
index 1d88f867971e8..1becd32cde968 100644
--- a/llvm/lib/Transforms/Utils/MisExpect.cpp
+++ b/llvm/lib/Transforms/Utils/MisExpect.cpp
@@ -108,6 +108,8 @@ uint64_t getScaledThreshold(const ProfDataSummary &PDS) {
bool isAnnotationDiagEnabled(LLVMContext &Ctx) {
LLVM_DEBUG(dbgs() << "PGOMissingAnnotations = " << PGOMissingAnnotations
<< "\n");
+ LLVM_DEBUG(dbgs() << "getAnnotationDiagsRequested = " << Ctx.getAnnotationDiagsRequested()
+ << "\n");
return PGOMissingAnnotations || Ctx.getAnnotationDiagsRequested();
}
@@ -304,10 +306,8 @@ void checkMissingAnnotations(Instruction &I,
if (IsFrontendInstr) {
verifyMissingAnnotations(I, ExistingWeights);
} else {
- SmallVector<uint32_t> ExpectedWeights;
- if (extractBranchWeights(I, ExpectedWeights))
- return;
- verifyMissingAnnotations(I, ExistingWeights);
+ if (!hasBranchWeightOrigin(I))
+ verifyMissingAnnotations(I, ExistingWeights);
}
}
``````````
</details>
https://github.com/llvm/llvm-project/pull/96525
More information about the llvm-branch-commits
mailing list