[PATCH] D73842: [xray][clang] Always add xray-skip-entry/exit and xray-ignore-loops attrs

[Shoaib Meenai via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG14f870366a93: [xray][clang] Always add xray-skip-entry/exit 
and xray-ignore-loops attrs (authored by ianlevesque, committed by smeenai).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D73842/new/

https://reviews.llvm.org/D73842

Files:
  clang/include/clang/Driver/XRayArgs.h
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/Driver/XRayArgs.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGen/xray-attributes-skip-entry-exit.cpp
  clang/test/CodeGen/xray-ignore-loops.cpp
  clang/test/Driver/XRay/xray-ignore-loops-flags.cpp

Index: clang/test/Driver/XRay/xray-ignore-loops-flags.cpp
===
--- /dev/null
+++ clang/test/Driver/XRay/xray-ignore-loops-flags.cpp
@@ -0,0 +1,10 @@
+// This test ensures that when we invoke the clang compiler, that the -cc1
+// options include the -fxray-ignore-loops flag we provide in the
+// invocation.
+//
+// RUN: %clang -fxray-instrument -fxray-ignore-loops -target x86_64-linux- -### \
+// RUN: -x c++ -std=c++11 -emit-llvm -c -o - %s 2>&1 \
+// RUN: | FileCheck %s
+// CHECK:  -fxray-ignore-loops
+//
+// REQUIRES: x86_64 || x86_64h
Index: clang/test/CodeGen/xray-ignore-loops.cpp
===
--- clang/test/CodeGen/xray-ignore-loops.cpp
+++ clang/test/CodeGen/xray-ignore-loops.cpp
@@ -4,5 +4,5 @@
   return 1;
 }
 
-// CHECK: define i32 @_Z3foov() #[[ATTRS:[0-9]+]] {
+// CHECK: define{{.*}} i32 @_Z3foov() #[[ATTRS:[0-9]+]] {
 // CHECK-DAG: attributes #[[ATTRS]] = {{.*}} "xray-ignore-loops" {{.*}}
Index: clang/test/CodeGen/xray-attributes-skip-entry-exit.cpp
===
--- /dev/null
+++ clang/test/CodeGen/xray-attributes-skip-entry-exit.cpp
@@ -0,0 +1,22 @@
+// RUN: %clang_cc1 -fxray-instrument \
+// RUN: -fxray-instrumentation-bundle=function-entry -x c++ \
+// RUN: -std=c++11 -triple x86_64-unknown-unknown -emit-llvm -o - %s \
+// RUN: | FileCheck --check-prefixes CHECK,SKIPEXIT %s
+// RUN: %clang_cc1 -fxray-instrument \
+// RUN: -fxray-instrumentation-bundle=function-exit -x c++ \
+// RUN: -std=c++11 -triple x86_64-unknown-unknown -emit-llvm -o - %s \
+// RUN: | FileCheck --check-prefixes CHECK,SKIPENTRY %s
+// RUN: %clang_cc1 -fxray-instrument \
+// RUN: -fxray-instrumentation-bundle=function-entry,function-exit -x c++ \
+// RUN: -std=c++11 -triple x86_64-unknown-unknown -emit-llvm -o - %s \
+// RUN: | FileCheck --check-prefixes CHECK,NOSKIPENTRY,NOSKIPEXIT %s
+
+// CHECK: define void @_Z13justAFunctionv() #[[ATTR:[0-9]+]] {
+void justAFunction() {
+}
+
+// SKIPENTRY: attributes #[[ATTR]] = {{.*}} "xray-skip-entry" {{.*}}
+// SKIPEXIT: attributes #[[ATTR]] = {{.*}} "xray-skip-exit" {{.*}}
+
+// NOSKIPENTRY-NOT: attributes #[[ATTR]] = {{.*}} "xray-skip-entry" {{.*}}
+// NOSKIPEXIT-NOT: attributes #[[ATTR]] = {{.*}} "xray-skip-exit" {{.*}}
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -1099,8 +1099,7 @@
   Args.hasArg(OPT_fxray_always_emit_typedevents);
   Opts.XRayInstructionThreshold =
   getLastArgIntValue(Args, OPT_fxray_instruction_threshold_EQ, 200, Diags);
-  Opts.XRayIgnoreLoops =
-  Args.hasArg(OPT_fxray_ignore_loops, OPT_fno_xray_ignore_loops, false);
+  Opts.XRayIgnoreLoops = Args.hasArg(OPT_fxray_ignore_loops);
 
   auto XRayInstrBundles =
   Args.getAllArgValues(OPT_fxray_instrumentation_bundle);
Index: clang/lib/Driver/XRayArgs.cpp
===
--- clang/lib/Driver/XRayArgs.cpp
+++ clang/lib/Driver/XRayArgs.cpp
@@ -101,6 +101,10 @@
 options::OPT_fnoxray_link_deps, true))
 XRayRT = false;
 
+  if (Args.hasFlag(options::OPT_fxray_ignore_loops,
+   options::OPT_fno_xray_ignore_loops, false))
+XRayIgnoreLoops = true;
+
   auto Bundles =
   Args.getAllArgValues(options::OPT_fxray_instrumentation_bundle);
   if (Bundles.empty())
@@ -197,6 +201,9 @@
   if (XRayAlwaysEmitTypedEvents)
 CmdArgs.push_back("-fxray-always-emit-typedevents");
 
+  if (XRayIgnoreLoops)
+CmdArgs.push_back("-fxray-ignore-loops");
+
   CmdArgs.push_back(Args.MakeArgString(Twine(XRayInstructionThresholdOption) +
Twine(InstructionThreshold)));
 
Index: clang/lib/CodeGen/CodeGenFunction.cpp
===
--- clang/lib/CodeGen/CodeGenFunction.cpp
+++ clang/lib/CodeGen/CodeGenFunction.cpp
@@ -814,23 +814,25 @@
   if (ShouldXRayInstrumentFunction())
 Fn->addFnAttr("xray-log-args",
   

[PATCH] D73842: [xray][clang] Always add xray-skip-entry/exit and xray-ignore-loops attrs

[Shoaib Meenai via Phabricator via cfe-commits]

smeenai accepted this revision.
smeenai added a comment.

LGTM. I'll commit this for you.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D73842/new/

https://reviews.llvm.org/D73842



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D73842: [xray][clang] Always add xray-skip-entry/exit and xray-ignore-loops attrs

[Ian Levesque via Phabricator via cfe-commits]

ianlevesque added a comment.

In D73842#1869890 , @smeenai wrote:

> Is it worth adding a test that a function with an explicit xray-instrument 
> attribute also has these other attributes applied?


The existing tests from when the feature was added covered that case well. It 
was the driver passing the flag and the case without the xray-instrument 
attribute that weren't working.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D73842/new/

https://reviews.llvm.org/D73842



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D73842: [xray][clang] Always add xray-skip-entry/exit and xray-ignore-loops attrs

[Ian Levesque via Phabricator via cfe-commits]

ianlevesque updated this revision to Diff 243981.
ianlevesque added a comment.

Address code review feedback on the tests.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D73842/new/

https://reviews.llvm.org/D73842

Files:
  clang/include/clang/Driver/XRayArgs.h
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/Driver/XRayArgs.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGen/xray-attributes-skip-entry-exit.cpp
  clang/test/CodeGen/xray-ignore-loops.cpp
  clang/test/Driver/XRay/xray-ignore-loops-flags.cpp

Index: clang/test/Driver/XRay/xray-ignore-loops-flags.cpp
===
--- /dev/null
+++ clang/test/Driver/XRay/xray-ignore-loops-flags.cpp
@@ -0,0 +1,10 @@
+// This test ensures that when we invoke the clang compiler, that the -cc1
+// options include the -fxray-ignore-loops flag we provide in the
+// invocation.
+//
+// RUN: %clang -fxray-instrument -fxray-ignore-loops -target x86_64-linux- -### \
+// RUN: -x c++ -std=c++11 -emit-llvm -c -o - %s 2>&1 \
+// RUN: | FileCheck %s
+// CHECK:  -fxray-ignore-loops
+//
+// REQUIRES: x86_64 || x86_64h
Index: clang/test/CodeGen/xray-ignore-loops.cpp
===
--- clang/test/CodeGen/xray-ignore-loops.cpp
+++ clang/test/CodeGen/xray-ignore-loops.cpp
@@ -4,5 +4,5 @@
   return 1;
 }
 
-// CHECK: define i32 @_Z3foov() #[[ATTRS:[0-9]+]] {
+// CHECK: define{{.*}} i32 @_Z3foov() #[[ATTRS:[0-9]+]] {
 // CHECK-DAG: attributes #[[ATTRS]] = {{.*}} "xray-ignore-loops" {{.*}}
Index: clang/test/CodeGen/xray-attributes-skip-entry-exit.cpp
===
--- /dev/null
+++ clang/test/CodeGen/xray-attributes-skip-entry-exit.cpp
@@ -0,0 +1,22 @@
+// RUN: %clang_cc1 -fxray-instrument \
+// RUN: -fxray-instrumentation-bundle=function-entry -x c++ \
+// RUN: -std=c++11 -triple x86_64-unknown-unknown -emit-llvm -o - %s \
+// RUN: | FileCheck --check-prefixes CHECK,SKIPEXIT %s
+// RUN: %clang_cc1 -fxray-instrument \
+// RUN: -fxray-instrumentation-bundle=function-exit -x c++ \
+// RUN: -std=c++11 -triple x86_64-unknown-unknown -emit-llvm -o - %s \
+// RUN: | FileCheck --check-prefixes CHECK,SKIPENTRY %s
+// RUN: %clang_cc1 -fxray-instrument \
+// RUN: -fxray-instrumentation-bundle=function-entry,function-exit -x c++ \
+// RUN: -std=c++11 -triple x86_64-unknown-unknown -emit-llvm -o - %s \
+// RUN: | FileCheck --check-prefixes CHECK,NOSKIPENTRY,NOSKIPEXIT %s
+
+// CHECK: define void @_Z13justAFunctionv() #[[ATTR:[0-9]+]] {
+void justAFunction() {
+}
+
+// SKIPENTRY: attributes #[[ATTR]] = {{.*}} "xray-skip-entry" {{.*}}
+// SKIPEXIT: attributes #[[ATTR]] = {{.*}} "xray-skip-exit" {{.*}}
+
+// NOSKIPENTRY-NOT: attributes #[[ATTR]] = {{.*}} "xray-skip-entry" {{.*}}
+// NOSKIPEXIT-NOT: attributes #[[ATTR]] = {{.*}} "xray-skip-exit" {{.*}}
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -1099,8 +1099,7 @@
   Args.hasArg(OPT_fxray_always_emit_typedevents);
   Opts.XRayInstructionThreshold =
   getLastArgIntValue(Args, OPT_fxray_instruction_threshold_EQ, 200, Diags);
-  Opts.XRayIgnoreLoops =
-  Args.hasArg(OPT_fxray_ignore_loops, OPT_fno_xray_ignore_loops, false);
+  Opts.XRayIgnoreLoops = Args.hasArg(OPT_fxray_ignore_loops);
 
   auto XRayInstrBundles =
   Args.getAllArgValues(OPT_fxray_instrumentation_bundle);
Index: clang/lib/Driver/XRayArgs.cpp
===
--- clang/lib/Driver/XRayArgs.cpp
+++ clang/lib/Driver/XRayArgs.cpp
@@ -101,6 +101,10 @@
 options::OPT_fnoxray_link_deps, true))
 XRayRT = false;
 
+  if (Args.hasFlag(options::OPT_fxray_ignore_loops,
+   options::OPT_fno_xray_ignore_loops, false))
+XRayIgnoreLoops = true;
+
   auto Bundles =
   Args.getAllArgValues(options::OPT_fxray_instrumentation_bundle);
   if (Bundles.empty())
@@ -197,6 +201,9 @@
   if (XRayAlwaysEmitTypedEvents)
 CmdArgs.push_back("-fxray-always-emit-typedevents");
 
+  if (XRayIgnoreLoops)
+CmdArgs.push_back("-fxray-ignore-loops");
+
   CmdArgs.push_back(Args.MakeArgString(Twine(XRayInstructionThresholdOption) +
Twine(InstructionThreshold)));
 
Index: clang/lib/CodeGen/CodeGenFunction.cpp
===
--- clang/lib/CodeGen/CodeGenFunction.cpp
+++ clang/lib/CodeGen/CodeGenFunction.cpp
@@ -814,23 +814,25 @@
   if (ShouldXRayInstrumentFunction())
 Fn->addFnAttr("xray-log-args",
   llvm::utostr(LogArgs->getArgumentCount()));
-if (!CGM.getCodeGenOpts().XRayInstrumentationBundle.has(
-

[PATCH] D73842: [xray][clang] Always add xray-skip-entry/exit and xray-ignore-loops attrs

[Shoaib Meenai via Phabricator via cfe-commits]

smeenai added a comment.

Is it worth adding a test that a function with an explicit xray-instrument 
attribute also has these other attributes applied?




Comment at: clang/test/CodeGen/xray-attributes-skip-entry-exit.cpp:8
+// RUN: -std=c++11 -triple x86_64-unknown-unknown -emit-llvm -o - %s \
+// RUN: | FileCheck --check-prefixes CHECK,NOCUSTOM,NOTYPED,SKIPENTRY %s
+// RUN: %clang_cc1 -fxray-instrument \

I don't see the NOCUSTOM or NOTYPED prefixes defined?



Comment at: clang/test/CodeGen/xray-attributes-skip-entry-exit.cpp:12
+// RUN: -std=c++11 -triple x86_64-unknown-unknown -emit-llvm -o - %s \
+// RUN: | FileCheck --check-prefixes CHECK,FUNCTION,NOCUSTOM,NOTYPED %s
+

Same with the FUNCTION prefix.



Comment at: clang/test/CodeGen/xray-ignore-loops.cpp:2
 // RUN: %clang_cc1 -fxray-instrument -fxray-ignore-loops -x c++ -std=c++11 
-emit-llvm -o - %s -triple x86_64-unknown-linux-gnu | FileCheck %s
+// RUN: %clang -fxray-instrument -fxray-ignore-loops -x c++ -std=c++11 -S 
-emit-llvm -o - %s -target x86_64-unknown-linux-gnu | FileCheck %s
 

From what I've seen, CodeGen tests don't usually invoke the driver directly. 
They have a test that the driver passes the correct flags to cc1 (as you do 
below), and then they just test that cc1 does the right thing with that flag.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D73842/new/

https://reviews.llvm.org/D73842



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D73842: [xray][clang] Always add xray-skip-entry/exit and xray-ignore-loops attrs

[Ian Levesque via Phabricator via cfe-commits]

ianlevesque updated this revision to Diff 243749.
ianlevesque added a comment.

Now with 100% more tests.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D73842/new/

https://reviews.llvm.org/D73842

Files:
  clang/include/clang/Driver/XRayArgs.h
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/Driver/XRayArgs.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGen/xray-attributes-skip-entry-exit.cpp
  clang/test/CodeGen/xray-ignore-loops.cpp
  clang/test/Driver/XRay/xray-ignore-loops-flags.cpp

Index: clang/test/Driver/XRay/xray-ignore-loops-flags.cpp
===
--- /dev/null
+++ clang/test/Driver/XRay/xray-ignore-loops-flags.cpp
@@ -0,0 +1,10 @@
+// This test ensures that when we invoke the clang compiler, that the -cc1
+// options include the -fxray-ignore-loops flag we provide in the
+// invocation.
+//
+// RUN: %clang -fxray-instrument -fxray-ignore-loops -target x86_64-linux- -### \
+// RUN: -x c++ -std=c++11 -emit-llvm -c -o - %s 2>&1 \
+// RUN: | FileCheck %s
+// CHECK:  -fxray-ignore-loops
+//
+// REQUIRES: x86_64 || x86_64h
\ No newline at end of file
Index: clang/test/CodeGen/xray-ignore-loops.cpp
===
--- clang/test/CodeGen/xray-ignore-loops.cpp
+++ clang/test/CodeGen/xray-ignore-loops.cpp
@@ -1,8 +1,9 @@
 // RUN: %clang_cc1 -fxray-instrument -fxray-ignore-loops -x c++ -std=c++11 -emit-llvm -o - %s -triple x86_64-unknown-linux-gnu | FileCheck %s
+// RUN: %clang -fxray-instrument -fxray-ignore-loops -x c++ -std=c++11 -S -emit-llvm -o - %s -target x86_64-unknown-linux-gnu | FileCheck %s
 
 int foo() {
   return 1;
 }
 
-// CHECK: define i32 @_Z3foov() #[[ATTRS:[0-9]+]] {
+// CHECK: define{{.*}} i32 @_Z3foov() #[[ATTRS:[0-9]+]] {
 // CHECK-DAG: attributes #[[ATTRS]] = {{.*}} "xray-ignore-loops" {{.*}}
Index: clang/test/CodeGen/xray-attributes-skip-entry-exit.cpp
===
--- /dev/null
+++ clang/test/CodeGen/xray-attributes-skip-entry-exit.cpp
@@ -0,0 +1,19 @@
+// RUN: %clang_cc1 -fxray-instrument \
+// RUN: -fxray-instrumentation-bundle=function-entry -x c++ \
+// RUN: -std=c++11 -triple x86_64-unknown-unknown -emit-llvm -o - %s \
+// RUN: | FileCheck --check-prefixes CHECK,NOCUSTOM,NOTYPED,SKIPEXIT %s
+// RUN: %clang_cc1 -fxray-instrument \
+// RUN: -fxray-instrumentation-bundle=function-exit -x c++ \
+// RUN: -std=c++11 -triple x86_64-unknown-unknown -emit-llvm -o - %s \
+// RUN: | FileCheck --check-prefixes CHECK,NOCUSTOM,NOTYPED,SKIPENTRY %s
+// RUN: %clang_cc1 -fxray-instrument \
+// RUN: -fxray-instrumentation-bundle=function-entry,function-exit -x c++ \
+// RUN: -std=c++11 -triple x86_64-unknown-unknown -emit-llvm -o - %s \
+// RUN: | FileCheck --check-prefixes CHECK,FUNCTION,NOCUSTOM,NOTYPED %s
+
+// CHECK: define void @_Z13justAFunctionv() #[[ATTR:[0-9]+]] {
+void justAFunction() {
+}
+
+// SKIPENTRY: attributes #[[ATTR]] = {{.*}} "xray-skip-entry" {{.*}}
+// SKIPEXIT: attributes #[[ATTR]] = {{.*}} "xray-skip-exit" {{.*}}
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -1099,8 +1099,7 @@
   Args.hasArg(OPT_fxray_always_emit_typedevents);
   Opts.XRayInstructionThreshold =
   getLastArgIntValue(Args, OPT_fxray_instruction_threshold_EQ, 200, Diags);
-  Opts.XRayIgnoreLoops =
-  Args.hasArg(OPT_fxray_ignore_loops, OPT_fno_xray_ignore_loops, false);
+  Opts.XRayIgnoreLoops = Args.hasArg(OPT_fxray_ignore_loops);
 
   auto XRayInstrBundles =
   Args.getAllArgValues(OPT_fxray_instrumentation_bundle);
Index: clang/lib/Driver/XRayArgs.cpp
===
--- clang/lib/Driver/XRayArgs.cpp
+++ clang/lib/Driver/XRayArgs.cpp
@@ -101,6 +101,10 @@
 options::OPT_fnoxray_link_deps, true))
 XRayRT = false;
 
+  if (Args.hasFlag(options::OPT_fxray_ignore_loops,
+   options::OPT_fno_xray_ignore_loops, false))
+XRayIgnoreLoops = true;
+
   auto Bundles =
   Args.getAllArgValues(options::OPT_fxray_instrumentation_bundle);
   if (Bundles.empty())
@@ -197,6 +201,9 @@
   if (XRayAlwaysEmitTypedEvents)
 CmdArgs.push_back("-fxray-always-emit-typedevents");
 
+  if (XRayIgnoreLoops)
+CmdArgs.push_back("-fxray-ignore-loops");
+
   CmdArgs.push_back(Args.MakeArgString(Twine(XRayInstructionThresholdOption) +
Twine(InstructionThreshold)));
 
Index: clang/lib/CodeGen/CodeGenFunction.cpp
===
--- clang/lib/CodeGen/CodeGenFunction.cpp
+++ clang/lib/CodeGen/CodeGenFunction.cpp
@@ -814,23 +814,25 @@
   if (ShouldXRayInstrumentFunction())
 

[PATCH] D73842: [xray][clang] Always add xray-skip-entry/exit and xray-ignore-loops attrs

[Shoaib Meenai via Phabricator via cfe-commits]

smeenai added a comment.

I can commit this for you, but is it possible to write a test case?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D73842/new/

https://reviews.llvm.org/D73842



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D73842: [xray][clang] Always add xray-skip-entry/exit and xray-ignore-loops attrs

[Ian Levesque via Phabricator via cfe-commits]

ianlevesque added a comment.

@hiraditya or @smeenai can i get a merge?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D73842/new/

https://reviews.llvm.org/D73842



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D73842: [xray][clang] Always add xray-skip-entry/exit and xray-ignore-loops attrs

[Ian Levesque via Phabricator via cfe-commits]

ianlevesque updated this revision to Diff 242800.
ianlevesque added a comment.

Address code review nits


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D73842/new/

https://reviews.llvm.org/D73842

Files:
  clang/include/clang/Driver/XRayArgs.h
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/Driver/XRayArgs.cpp
  clang/lib/Frontend/CompilerInvocation.cpp


Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -1099,8 +1099,7 @@
   Args.hasArg(OPT_fxray_always_emit_typedevents);
   Opts.XRayInstructionThreshold =
   getLastArgIntValue(Args, OPT_fxray_instruction_threshold_EQ, 200, Diags);
-  Opts.XRayIgnoreLoops =
-  Args.hasArg(OPT_fxray_ignore_loops, OPT_fno_xray_ignore_loops, false);
+  Opts.XRayIgnoreLoops = Args.hasArg(OPT_fxray_ignore_loops);
 
   auto XRayInstrBundles =
   Args.getAllArgValues(OPT_fxray_instrumentation_bundle);
Index: clang/lib/Driver/XRayArgs.cpp
===
--- clang/lib/Driver/XRayArgs.cpp
+++ clang/lib/Driver/XRayArgs.cpp
@@ -101,6 +101,10 @@
 options::OPT_fnoxray_link_deps, true))
 XRayRT = false;
 
+  if (Args.hasFlag(options::OPT_fxray_ignore_loops,
+   options::OPT_fno_xray_ignore_loops, false))
+XRayIgnoreLoops = true;
+
   auto Bundles =
   Args.getAllArgValues(options::OPT_fxray_instrumentation_bundle);
   if (Bundles.empty())
@@ -197,6 +201,9 @@
   if (XRayAlwaysEmitTypedEvents)
 CmdArgs.push_back("-fxray-always-emit-typedevents");
 
+  if (XRayIgnoreLoops)
+CmdArgs.push_back("-fxray-ignore-loops");
+
   CmdArgs.push_back(Args.MakeArgString(Twine(XRayInstructionThresholdOption) +
Twine(InstructionThreshold)));
 
Index: clang/lib/CodeGen/CodeGenFunction.cpp
===
--- clang/lib/CodeGen/CodeGenFunction.cpp
+++ clang/lib/CodeGen/CodeGenFunction.cpp
@@ -814,23 +814,25 @@
   if (ShouldXRayInstrumentFunction())
 Fn->addFnAttr("xray-log-args",
   llvm::utostr(LogArgs->getArgumentCount()));
-if (!CGM.getCodeGenOpts().XRayInstrumentationBundle.has(
-XRayInstrKind::FunctionExit)) {
-  Fn->addFnAttr("xray-skip-exit");
-}
-if (!CGM.getCodeGenOpts().XRayInstrumentationBundle.has(
-XRayInstrKind::FunctionEntry)) {
-  Fn->addFnAttr("xray-skip-entry");
-}
   }
 } else {
   if (ShouldXRayInstrumentFunction() && !CGM.imbueXRayAttrs(Fn, Loc))
 Fn->addFnAttr(
 "xray-instruction-threshold",
 llvm::itostr(CGM.getCodeGenOpts().XRayInstructionThreshold));
-  if (CGM.getCodeGenOpts().XRayIgnoreLoops) {
+}
+
+if (ShouldXRayInstrumentFunction()) {
+  if (CGM.getCodeGenOpts().XRayIgnoreLoops)
 Fn->addFnAttr("xray-ignore-loops");
-  }
+
+  if (!CGM.getCodeGenOpts().XRayInstrumentationBundle.has(
+  XRayInstrKind::FunctionExit))
+Fn->addFnAttr("xray-skip-exit");
+
+  if (!CGM.getCodeGenOpts().XRayInstrumentationBundle.has(
+  XRayInstrKind::FunctionEntry))
+Fn->addFnAttr("xray-skip-entry");
 }
 
 unsigned Count, Offset;
Index: clang/include/clang/Driver/XRayArgs.h
===
--- clang/include/clang/Driver/XRayArgs.h
+++ clang/include/clang/Driver/XRayArgs.h
@@ -30,6 +30,7 @@
   bool XRayAlwaysEmitCustomEvents = false;
   bool XRayAlwaysEmitTypedEvents = false;
   bool XRayRT = true;
+  bool XRayIgnoreLoops = false;
 
 public:
   /// Parses the XRay arguments from an argument list.


Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -1099,8 +1099,7 @@
   Args.hasArg(OPT_fxray_always_emit_typedevents);
   Opts.XRayInstructionThreshold =
   getLastArgIntValue(Args, OPT_fxray_instruction_threshold_EQ, 200, Diags);
-  Opts.XRayIgnoreLoops =
-  Args.hasArg(OPT_fxray_ignore_loops, OPT_fno_xray_ignore_loops, false);
+  Opts.XRayIgnoreLoops = Args.hasArg(OPT_fxray_ignore_loops);
 
   auto XRayInstrBundles =
   Args.getAllArgValues(OPT_fxray_instrumentation_bundle);
Index: clang/lib/Driver/XRayArgs.cpp
===
--- clang/lib/Driver/XRayArgs.cpp
+++ clang/lib/Driver/XRayArgs.cpp
@@ -101,6 +101,10 @@
 options::OPT_fnoxray_link_deps, true))
 XRayRT = false;
 
+  if (Args.hasFlag(options::OPT_fxray_ignore_loops,
+   options::OPT_fno_xray_ignore_loops, false))
+XRayIgnoreLoops = true;
+
   auto Bundles =
   

[PATCH] D73842: [xray][clang] Always add xray-skip-entry/exit and xray-ignore-loops attrs

[Ian Levesque via Phabricator via cfe-commits]

ianlevesque marked 2 inline comments as done.
ianlevesque added inline comments.



Comment at: clang/lib/CodeGen/CodeGenFunction.cpp:832
+Fn->addFnAttr("xray-skip-exit");
+  }
+  if (!CGM.getCodeGenOpts().XRayInstrumentationBundle.has(

@hiraditya I realize on a previous diff you suggested dropping the brackets. 
I'll do that here too.



Comment at: clang/lib/Frontend/CompilerInvocation.cpp:1103
   Opts.XRayIgnoreLoops =
-  Args.hasArg(OPT_fxray_ignore_loops, OPT_fno_xray_ignore_loops, false);
+  Args.hasFlag(OPT_fxray_ignore_loops, OPT_fno_xray_ignore_loops, false);
 

MaskRay wrote:
> `Args.hasArg(OPT_fxray_ignore_loops);`
> 
> There is no need to check a fno option in cc1, if the option is always 
> enabled/disabled by default.
Will update.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D73842/new/

https://reviews.llvm.org/D73842



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D73842: [xray][clang] Always add xray-skip-entry/exit and xray-ignore-loops attrs

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added inline comments.



Comment at: clang/lib/Frontend/CompilerInvocation.cpp:1103
   Opts.XRayIgnoreLoops =
-  Args.hasArg(OPT_fxray_ignore_loops, OPT_fno_xray_ignore_loops, false);
+  Args.hasFlag(OPT_fxray_ignore_loops, OPT_fno_xray_ignore_loops, false);
 

`Args.hasArg(OPT_fxray_ignore_loops);`

There is no need to check a fno option in cc1, if the option is always 
enabled/disabled by default.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D73842/new/

https://reviews.llvm.org/D73842



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D73842: [xray][clang] Always add xray-skip-entry/exit and xray-ignore-loops attrs

[Ian Levesque via Phabricator via cfe-commits]

ianlevesque created this revision.
ianlevesque added reviewers: hiraditya, smeenai, dberris.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

The function attributes xray-skip-entry, xray-skip-exit, and
xray-ignore-loops were only being applied if a function had an
xray-instrument attribute, but they should apply if xray is enabled
globally too.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D73842

Files:
  clang/include/clang/Driver/XRayArgs.h
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/Driver/XRayArgs.cpp
  clang/lib/Frontend/CompilerInvocation.cpp


Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -1100,7 +1100,7 @@
   Opts.XRayInstructionThreshold =
   getLastArgIntValue(Args, OPT_fxray_instruction_threshold_EQ, 200, Diags);
   Opts.XRayIgnoreLoops =
-  Args.hasArg(OPT_fxray_ignore_loops, OPT_fno_xray_ignore_loops, false);
+  Args.hasFlag(OPT_fxray_ignore_loops, OPT_fno_xray_ignore_loops, false);
 
   auto XRayInstrBundles =
   Args.getAllArgValues(OPT_fxray_instrumentation_bundle);
Index: clang/lib/Driver/XRayArgs.cpp
===
--- clang/lib/Driver/XRayArgs.cpp
+++ clang/lib/Driver/XRayArgs.cpp
@@ -101,6 +101,10 @@
 options::OPT_fnoxray_link_deps, true))
 XRayRT = false;
 
+  if (Args.hasFlag(options::OPT_fxray_ignore_loops,
+   options::OPT_fno_xray_ignore_loops, false))
+XRayIgnoreLoops = true;
+
   auto Bundles =
   Args.getAllArgValues(options::OPT_fxray_instrumentation_bundle);
   if (Bundles.empty())
@@ -197,6 +201,9 @@
   if (XRayAlwaysEmitTypedEvents)
 CmdArgs.push_back("-fxray-always-emit-typedevents");
 
+  if (XRayIgnoreLoops)
+CmdArgs.push_back("-fxray-ignore-loops");
+
   CmdArgs.push_back(Args.MakeArgString(Twine(XRayInstructionThresholdOption) +
Twine(InstructionThreshold)));
 
Index: clang/lib/CodeGen/CodeGenFunction.cpp
===
--- clang/lib/CodeGen/CodeGenFunction.cpp
+++ clang/lib/CodeGen/CodeGenFunction.cpp
@@ -814,23 +814,26 @@
   if (ShouldXRayInstrumentFunction())
 Fn->addFnAttr("xray-log-args",
   llvm::utostr(LogArgs->getArgumentCount()));
-if (!CGM.getCodeGenOpts().XRayInstrumentationBundle.has(
-XRayInstrKind::FunctionExit)) {
-  Fn->addFnAttr("xray-skip-exit");
-}
-if (!CGM.getCodeGenOpts().XRayInstrumentationBundle.has(
-XRayInstrKind::FunctionEntry)) {
-  Fn->addFnAttr("xray-skip-entry");
-}
   }
 } else {
   if (ShouldXRayInstrumentFunction() && !CGM.imbueXRayAttrs(Fn, Loc))
 Fn->addFnAttr(
 "xray-instruction-threshold",
 llvm::itostr(CGM.getCodeGenOpts().XRayInstructionThreshold));
+}
+
+if (ShouldXRayInstrumentFunction()) {
   if (CGM.getCodeGenOpts().XRayIgnoreLoops) {
 Fn->addFnAttr("xray-ignore-loops");
   }
+  if (!CGM.getCodeGenOpts().XRayInstrumentationBundle.has(
+  XRayInstrKind::FunctionExit)) {
+Fn->addFnAttr("xray-skip-exit");
+  }
+  if (!CGM.getCodeGenOpts().XRayInstrumentationBundle.has(
+  XRayInstrKind::FunctionEntry)) {
+Fn->addFnAttr("xray-skip-entry");
+  }
 }
 
 unsigned Count, Offset;
Index: clang/include/clang/Driver/XRayArgs.h
===
--- clang/include/clang/Driver/XRayArgs.h
+++ clang/include/clang/Driver/XRayArgs.h
@@ -30,6 +30,7 @@
   bool XRayAlwaysEmitCustomEvents = false;
   bool XRayAlwaysEmitTypedEvents = false;
   bool XRayRT = true;
+  bool XRayIgnoreLoops = false;
 
 public:
   /// Parses the XRay arguments from an argument list.


Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -1100,7 +1100,7 @@
   Opts.XRayInstructionThreshold =
   getLastArgIntValue(Args, OPT_fxray_instruction_threshold_EQ, 200, Diags);
   Opts.XRayIgnoreLoops =
-  Args.hasArg(OPT_fxray_ignore_loops, OPT_fno_xray_ignore_loops, false);
+  Args.hasFlag(OPT_fxray_ignore_loops, OPT_fno_xray_ignore_loops, false);
 
   auto XRayInstrBundles =
   Args.getAllArgValues(OPT_fxray_instrumentation_bundle);
Index: clang/lib/Driver/XRayArgs.cpp
===
--- clang/lib/Driver/XRayArgs.cpp
+++ clang/lib/Driver/XRayArgs.cpp
@@ -101,6 +101,10 @@
 options::OPT_fnoxray_link_deps, true))
 XRayRT = false;
 
+  if (Args.hasFlag(options::OPT_fxray_ignore_loops,
+