hans added inline comments.
================
Comment at: clang/include/clang/Driver/Options.td:1253
+ Flags<[NoXarchOption, CC1Option]>, Group<f_Group>,
+ HelpText<"Use profi to infer block and edge counts.">;
def fno_profile_sample_accurate : Flag<["-"], "fno-profile-sample-accurate">,
----------------
A user who reads the help message might reasonably ask "what's profi". Can you
please add some documentation about it? It would also be nice with a DocBrief
(see e.g. -fprofile-sample-accurate above).
================
Comment at: clang/lib/Driver/ToolChains/Clang.cpp:5763
+ // Enable profi in frontend and forward to LLVM invocation
+ if (Args.hasArg(options::OPT_fsample_profile_use_profi)) {
----------------
Perhaps consider skipping this comment, the code is clear enough.
================
Comment at: clang/test/CodeGen/pgo-sample-use-profi.cpp:1
+//Test if profi flat is enabled in frontend as user-facing feature.
+//
----------------
HaoyuZhang wrote:
> MaskRay wrote:
> > Remove the test.
> >
> > This change only needs a clang/test/Driver test showing that the option
> > passes `-sample-profile-use-profi` to -cc1.
> Thank you for the comments. So we only need to RUN the command for example:
>
> `//RUN: %clang_cc1 %s -fsample-profile-use-profi -fdebug-pass-manager
> -emit-llvm -o - 2>&1 | FileCheck %s`
>
> `//CHECK: Running pass: VerifierPass`
>
> and without any test code.
>
I think what MaskRay is saying is that you only need a test in
clang/test/Driver/ that does "%clang -c -fsample-profile-use-profi -###" and
FileCheck's that the cc1 invocation contains the right -mllvm flag.
(I assume you have tests for the actual functionality somewhere in llvm/test/)
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D136846/new/
https://reviews.llvm.org/D136846
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
