MaskRay added a comment.
The idea seems fine.
> By selecting different groups over time you could cover the entire
> application incrementally with lower overhead than instrumenting the entire
> application at once.
How large the overhead is? This is somewhat surprising to me.
================
Comment at: clang/include/clang/Driver/Options.td:1343
+def fxray_function_groups :
+ JoinedOrSeparate<["-"], "fxray-function-groups=">,
+ Group<f_Group>, Flags<[CC1Option]>,
----------------
Joined.
Separate is for a space between the option name and its value.
================
Comment at: clang/include/clang/Driver/Options.td:1345
+ Group<f_Group>, Flags<[CC1Option]>,
+ HelpText<"Only instrument 1 of N functions">;
+
----------------
It is "N groups", not "N functions".
================
Comment at: clang/lib/CodeGen/CodeGenFunction.cpp:814
+ auto FuncGroups = CGM.getCodeGenOpts().XRayTotalFunctionGroups;
+ if (FuncGroups > 1) {
+ auto FuncName = ArrayRef<uint8_t>(CurFn->getName().bytes_begin(),
----------------
For one group, the branch is skipped, which does not seem correct
================
Comment at: clang/lib/CodeGen/CodeGenFunction.cpp:815
+ if (FuncGroups > 1) {
+ auto FuncName = ArrayRef<uint8_t>(CurFn->getName().bytes_begin(),
+ CurFn->getName().bytes_end());
----------------
`const ArrayRef<uint8_t> FuncName(..., ...)`
================
Comment at: clang/lib/CodeGen/CodeGenFunction.cpp:820
+ !AlwaysXRayAttr) {
+ Fn->addFnAttr("function-instrument", "xray-never");
+ }
----------------
Omit braces around one-line simple statements.
================
Comment at: clang/test/CodeGen/xray-function-groups.cpp:4
+// RUN: -fxray-selected-function-group=0 \
+// RUN: -x c++ -std=c++11 -emit-llvm -o - %s \
+// RUN: -triple x86_64-unknown-linux-gnu | FileCheck
--check-prefix=GROUP0 %s
----------------
You can omit `-x c++ -std=c++11 `. They are insignificant.
Then consider packing more options on one line to reduce the total number of
lines.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D87953/new/
https://reviews.llvm.org/D87953
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
