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