[PATCH] D24799: [XRay] Check in Clang whether XRay supports the target when -fxray-instrument is passed

[Serge Rogatch via cfe-commits]

rSerge created this revision.
rSerge added reviewers: rsmith, dberris.
rSerge added subscribers: cfe-commits, iid_iunknown.
Herald added a subscriber: dberris.

Added the code which explicitly emits an error in Clang in case 
`-fxray-instrument` is passed, but XRay is not supported for the selected 
target.

https://reviews.llvm.org/D24799

Files:
  lib/Driver/Tools.cpp

Index: lib/Driver/Tools.cpp
===
--- lib/Driver/Tools.cpp
+++ lib/Driver/Tools.cpp
@@ -4767,6 +4767,17 @@
 
   if (Args.hasFlag(options::OPT_fxray_instrument,
options::OPT_fnoxray_instrument, false)) {
+switch(getToolChain().getArch()) {
+case llvm::Triple::arm:
+case llvm::Triple::x86_64:
+  break;
+default:
+{
+  std::string feature("XRay for ");
+  feature += Triple.getArchName().data();
+  D.Diag(diag::err_drv_clang_unsupported) << feature;
+  break;
+} }
 CmdArgs.push_back("-fxray-instrument");
 if (const Arg *A =
 Args.getLastArg(options::OPT_fxray_instruction_threshold_,


Index: lib/Driver/Tools.cpp
===
--- lib/Driver/Tools.cpp
+++ lib/Driver/Tools.cpp
@@ -4767,6 +4767,17 @@
 
   if (Args.hasFlag(options::OPT_fxray_instrument,
options::OPT_fnoxray_instrument, false)) {
+switch(getToolChain().getArch()) {
+case llvm::Triple::arm:
+case llvm::Triple::x86_64:
+  break;
+default:
+{
+  std::string feature("XRay for ");
+  feature += Triple.getArchName().data();
+  D.Diag(diag::err_drv_clang_unsupported) << feature;
+  break;
+} }
 CmdArgs.push_back("-fxray-instrument");
 if (const Arg *A =
 Args.getLastArg(options::OPT_fxray_instruction_threshold_,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D24799: [XRay] Check in Clang whether XRay supports the target when -fxray-instrument is passed

[Serge Rogatch via cfe-commits]

rSerge added a comment.

In https://reviews.llvm.org/D24799#549442, @dberris wrote:

> What does the error actually look like? Can you add a test for it? It's 
> unclear to me how this would read... for example does it say "XRay for arm is 
> unsupported"?


In the attached picture you can see how the error looks when cross-compiling 
with Clang from x86_64-Windows host to Thumb-Linux target. 
F2439489: Unsupported XRay target error.jpg 
It says

> clang++.exe: error: the clang compiler does not support 'XRay for 
> armv6kz--linux-gnueabihf'


I'll try to add tests.


https://reviews.llvm.org/D24799



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

Re: [PATCH] D24799: [XRay] Check in Clang whether XRay supports the target when -fxray-instrument is passed

[Serge Rogatch via cfe-commits]

rSerge updated this revision to Diff 72312.
rSerge added a comment.
Herald added a subscriber: rampitec.

Added a test.
Changed the error message to:

> clang++.exe: error: the clang compiler does not support '-fxray-instrument on 
> armv6kz--linux-gnueabihf'



https://reviews.llvm.org/D24799

Files:
  lib/Driver/Tools.cpp
  test/Driver/xray-instrument.c

Index: test/Driver/xray-instrument.c
===
--- test/Driver/xray-instrument.c
+++ test/Driver/xray-instrument.c
@@ -0,0 +1,2 @@
+// RUN: %clang -fxray-instrument %s
+// XFAIL: 
armeb,aarch64,aarch64_be,avr,bpfel,bpfeb,hexagon,mips,mipsel,mips64,mips64el,msp430,ppc,ppc64,ppc64le,r600,amdgcn,sparc,sparcv9,sparcel,systemz,tce,thumb,thumbeb,x86,xcore,nvptx,nvptx64,le32,le64,amdil,amdil64,hsail,hsail64,spir,spir64,kalimba,shave,lanai,wasm32,wasm64,renderscript32,renderscript64
Index: lib/Driver/Tools.cpp
===
--- lib/Driver/Tools.cpp
+++ lib/Driver/Tools.cpp
@@ -4767,7 +4767,20 @@
 
   if (Args.hasFlag(options::OPT_fxray_instrument,
options::OPT_fnoxray_instrument, false)) {
-CmdArgs.push_back("-fxray-instrument");
+const char* const XRayInstrumentOption = "-fxray-instrument";
+switch(getToolChain().getArch()) {
+case llvm::Triple::arm:
+case llvm::Triple::x86_64:
+  break;
+default:
+{
+  std::string Feature(XRayInstrumentOption);
+  Feature += " on ";
+  Feature += Triple.getArchName().data();
+  D.Diag(diag::err_drv_clang_unsupported) << Feature;
+  break;
+} }
+CmdArgs.push_back(XRayInstrumentOption);
 if (const Arg *A =
 Args.getLastArg(options::OPT_fxray_instruction_threshold_,
 options::OPT_fxray_instruction_threshold_EQ)) {


Index: test/Driver/xray-instrument.c
===
--- test/Driver/xray-instrument.c
+++ test/Driver/xray-instrument.c
@@ -0,0 +1,2 @@
+// RUN: %clang -fxray-instrument %s
+// XFAIL: armeb,aarch64,aarch64_be,avr,bpfel,bpfeb,hexagon,mips,mipsel,mips64,mips64el,msp430,ppc,ppc64,ppc64le,r600,amdgcn,sparc,sparcv9,sparcel,systemz,tce,thumb,thumbeb,x86,xcore,nvptx,nvptx64,le32,le64,amdil,amdil64,hsail,hsail64,spir,spir64,kalimba,shave,lanai,wasm32,wasm64,renderscript32,renderscript64
Index: lib/Driver/Tools.cpp
===
--- lib/Driver/Tools.cpp
+++ lib/Driver/Tools.cpp
@@ -4767,7 +4767,20 @@
 
   if (Args.hasFlag(options::OPT_fxray_instrument,
options::OPT_fnoxray_instrument, false)) {
-CmdArgs.push_back("-fxray-instrument");
+const char* const XRayInstrumentOption = "-fxray-instrument";
+switch(getToolChain().getArch()) {
+case llvm::Triple::arm:
+case llvm::Triple::x86_64:
+  break;
+default:
+{
+  std::string Feature(XRayInstrumentOption);
+  Feature += " on ";
+  Feature += Triple.getArchName().data();
+  D.Diag(diag::err_drv_clang_unsupported) << Feature;
+  break;
+} }
+CmdArgs.push_back(XRayInstrumentOption);
 if (const Arg *A =
 Args.getLastArg(options::OPT_fxray_instruction_threshold_,
 options::OPT_fxray_instruction_threshold_EQ)) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D24799: [XRay] Check in Clang whether XRay supports the target when -fxray-instrument is passed

[Serge Rogatch via cfe-commits]

rSerge added inline comments.


Comment at: lib/Driver/Tools.cpp:4777-4780
@@ +4776,6 @@
+{
+  std::string Feature(XRayInstrumentOption);
+  Feature += " on ";
+  Feature += Triple.getArchName().data();
+  D.Diag(diag::err_drv_clang_unsupported) << Feature;
+  break;

dberris wrote:
> Wouldn't something like this work better:
> 
>   D.Diag(...) << XRayInstrumentOption << " on " << Triple.getArchName();
No, it doesn't work this way.  Although `D.Diag` looks like a C++ stream 
because of overloaded `<<` operator, it works like C `printf` and 
`diag::err_drv_clang_unsupported` is like a format specifier. The latter 
specifies that only the first argument added via `operator<<` is included in 
the final message. So we have to pass in one string everything we want to say. 
And LLVM coding guidelines forbid C++ streams.


https://reviews.llvm.org/D24799



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

Re: [PATCH] D24799: [XRay] Check in Clang whether XRay supports the target when -fxray-instrument is passed

[Serge Rogatch via cfe-commits]

rSerge updated this revision to Diff 72554.
rSerge added a comment.

Implemented a workaround for XFAIL not differentiating between `x86` and 
`x86_64` because it searches for a substring in the triple string, thus `x86` 
matches both `x86-X-Y-Z` and `x86_64-X-Y-Z`.


https://reviews.llvm.org/D24799

Files:
  lib/Driver/Tools.cpp
  test/Driver/xray-instrument.c

Index: test/Driver/xray-instrument.c
===
--- test/Driver/xray-instrument.c
+++ test/Driver/xray-instrument.c
@@ -0,0 +1,3 @@
+// RUN: %clang -v -fxray-instrument -c %s
+// XFAIL: armeb, aarch64, aarch64_be, avr, bpfel, bpfeb, hexagon, mips, 
mipsel, mips64, mips64el, msp430, ppc, ppc64, ppc64le, r600, amdgcn, sparc, 
sparcv9, sparcel, systemz, tce, thumb, thumbeb, x86-, xcore, nvptx, nvptx64, 
le32, le64, amdil, amdil64, hsail, hsail64, spir, spir64, kalimba, shave, 
lanai, wasm32, wasm64, renderscript32, renderscript64
+typedef int a;
Index: lib/Driver/Tools.cpp
===
--- lib/Driver/Tools.cpp
+++ lib/Driver/Tools.cpp
@@ -4767,7 +4767,20 @@
 
   if (Args.hasFlag(options::OPT_fxray_instrument,
options::OPT_fnoxray_instrument, false)) {
-CmdArgs.push_back("-fxray-instrument");
+const char* const XRayInstrumentOption = "-fxray-instrument";
+switch(getToolChain().getArch()) {
+case llvm::Triple::arm:
+case llvm::Triple::x86_64:
+  break;
+default:
+{
+  std::string Feature(XRayInstrumentOption);
+  Feature += " on ";
+  Feature += Triple.getArchName().data();
+  D.Diag(diag::err_drv_clang_unsupported) << Feature;
+  break;
+} }
+CmdArgs.push_back(XRayInstrumentOption);
 if (const Arg *A =
 Args.getLastArg(options::OPT_fxray_instruction_threshold_,
 options::OPT_fxray_instruction_threshold_EQ)) {


Index: test/Driver/xray-instrument.c
===
--- test/Driver/xray-instrument.c
+++ test/Driver/xray-instrument.c
@@ -0,0 +1,3 @@
+// RUN: %clang -v -fxray-instrument -c %s
+// XFAIL: armeb, aarch64, aarch64_be, avr, bpfel, bpfeb, hexagon, mips, mipsel, mips64, mips64el, msp430, ppc, ppc64, ppc64le, r600, amdgcn, sparc, sparcv9, sparcel, systemz, tce, thumb, thumbeb, x86-, xcore, nvptx, nvptx64, le32, le64, amdil, amdil64, hsail, hsail64, spir, spir64, kalimba, shave, lanai, wasm32, wasm64, renderscript32, renderscript64
+typedef int a;
Index: lib/Driver/Tools.cpp
===
--- lib/Driver/Tools.cpp
+++ lib/Driver/Tools.cpp
@@ -4767,7 +4767,20 @@
 
   if (Args.hasFlag(options::OPT_fxray_instrument,
options::OPT_fnoxray_instrument, false)) {
-CmdArgs.push_back("-fxray-instrument");
+const char* const XRayInstrumentOption = "-fxray-instrument";
+switch(getToolChain().getArch()) {
+case llvm::Triple::arm:
+case llvm::Triple::x86_64:
+  break;
+default:
+{
+  std::string Feature(XRayInstrumentOption);
+  Feature += " on ";
+  Feature += Triple.getArchName().data();
+  D.Diag(diag::err_drv_clang_unsupported) << Feature;
+  break;
+} }
+CmdArgs.push_back(XRayInstrumentOption);
 if (const Arg *A =
 Args.getLastArg(options::OPT_fxray_instruction_threshold_,
 options::OPT_fxray_instruction_threshold_EQ)) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D24799: [XRay] Check in Clang whether XRay supports the target when -fxray-instrument is passed

[Serge Rogatch via cfe-commits]

rSerge added a comment.

@dberris ,  could you deliver this patch to mainline? Or do we need approval 
from more reviewers?


https://reviews.llvm.org/D24799



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

[PATCH] D24799: [XRay] Check in Clang whether XRay supports the target when -fxray-instrument is passed

[Serge Rogatch via cfe-commits]

rSerge removed rL LLVM as the repository for this revision.
rSerge updated this revision to Diff 73509.
rSerge added a comment.

My mistake was that initially I only enumerated the unsupported targets from 
llvm\include\llvm\ADT\Triple.h . Now I've added also the cases from 
llvm\lib\Support\Triple.cpp .
`XFAIL` requires a list of all unsupported cases, which is currently much 
larger than the list of supported cases. However, AFAIK there is nothing like 
`XPASS` in LIT.


https://reviews.llvm.org/D24799

Files:
  lib/Driver/Tools.cpp
  test/Driver/xray-instrument.c


Index: test/Driver/xray-instrument.c
===
--- test/Driver/xray-instrument.c
+++ test/Driver/xray-instrument.c
@@ -0,0 +1,3 @@
+// RUN: %clang -v -fxray-instrument -c %s
+// XFAIL: armeb-, aarch64-, aarch64_be-, avr-, bpfel-, bpfeb-, hexagon-, 
mips-, mipsel-, mips64-, mips64el-, msp430-, ppc-, ppc64-, ppc64le-, r600-, 
amdgcn-, sparc-, sparcv9-, sparcel-, systemz-, tce-, thumb-, thumbeb-, x86-, 
xcore-, nvptx-, nvptx64-, le32-, le64-, amdil-, amdil64-, hsail-, hsail64-, 
spir-, spir64-, kalimba-, shave-, lanai-, wasm32-, wasm64-, renderscript32-, 
renderscript64-, i386-, i486-, i586-, i686-, i786-, i886-, i986-, powerpc-, 
ppc32-, powerpc64-, ppu-, powerpc64le-, ppc64le-, xscaleeb-, arm64-, mipseb-, 
mipsallegrex-, mipsallegrexel-, mips64eb-, s390x-, sparc64-
+typedef int a;
Index: lib/Driver/Tools.cpp
===
--- lib/Driver/Tools.cpp
+++ lib/Driver/Tools.cpp
@@ -4784,7 +4784,20 @@
 
   if (Args.hasFlag(options::OPT_fxray_instrument,
options::OPT_fnoxray_instrument, false)) {
-CmdArgs.push_back("-fxray-instrument");
+const char* const XRayInstrumentOption = "-fxray-instrument";
+switch(getToolChain().getArch()) {
+case llvm::Triple::arm:
+case llvm::Triple::x86_64:
+  break;
+default:
+{
+  std::string Feature(XRayInstrumentOption);
+  Feature += " on ";
+  Feature += Triple.getArchName().data();
+  D.Diag(diag::err_drv_clang_unsupported) << Feature;
+  break;
+} }
+CmdArgs.push_back(XRayInstrumentOption);
 if (const Arg *A =
 Args.getLastArg(options::OPT_fxray_instruction_threshold_,
 options::OPT_fxray_instruction_threshold_EQ)) {


Index: test/Driver/xray-instrument.c
===
--- test/Driver/xray-instrument.c
+++ test/Driver/xray-instrument.c
@@ -0,0 +1,3 @@
+// RUN: %clang -v -fxray-instrument -c %s
+// XFAIL: armeb-, aarch64-, aarch64_be-, avr-, bpfel-, bpfeb-, hexagon-, mips-, mipsel-, mips64-, mips64el-, msp430-, ppc-, ppc64-, ppc64le-, r600-, amdgcn-, sparc-, sparcv9-, sparcel-, systemz-, tce-, thumb-, thumbeb-, x86-, xcore-, nvptx-, nvptx64-, le32-, le64-, amdil-, amdil64-, hsail-, hsail64-, spir-, spir64-, kalimba-, shave-, lanai-, wasm32-, wasm64-, renderscript32-, renderscript64-, i386-, i486-, i586-, i686-, i786-, i886-, i986-, powerpc-, ppc32-, powerpc64-, ppu-, powerpc64le-, ppc64le-, xscaleeb-, arm64-, mipseb-, mipsallegrex-, mipsallegrexel-, mips64eb-, s390x-, sparc64-
+typedef int a;
Index: lib/Driver/Tools.cpp
===
--- lib/Driver/Tools.cpp
+++ lib/Driver/Tools.cpp
@@ -4784,7 +4784,20 @@
 
   if (Args.hasFlag(options::OPT_fxray_instrument,
options::OPT_fnoxray_instrument, false)) {
-CmdArgs.push_back("-fxray-instrument");
+const char* const XRayInstrumentOption = "-fxray-instrument";
+switch(getToolChain().getArch()) {
+case llvm::Triple::arm:
+case llvm::Triple::x86_64:
+  break;
+default:
+{
+  std::string Feature(XRayInstrumentOption);
+  Feature += " on ";
+  Feature += Triple.getArchName().data();
+  D.Diag(diag::err_drv_clang_unsupported) << Feature;
+  break;
+} }
+CmdArgs.push_back(XRayInstrumentOption);
 if (const Arg *A =
 Args.getLastArg(options::OPT_fxray_instruction_threshold_,
 options::OPT_fxray_instruction_threshold_EQ)) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D24799: [XRay] Check in Clang whether XRay supports the target when -fxray-instrument is passed

[Serge Rogatch via cfe-commits]

rSerge added a comment.

In https://reviews.llvm.org/D24799#561879, @dberris wrote:

> In https://reviews.llvm.org/D24799#561106, @rSerge wrote:
>
> > My mistake was that initially I only enumerated the unsupported targets 
> > from llvm\include\llvm\ADT\Triple.h . Now I've added also the cases from 
> > llvm\lib\Support\Triple.cpp .
> >  `XFAIL` requires a list of all unsupported cases, which is currently much 
> > larger than the list of supported cases. However, AFAIK there is nothing 
> > like `XPASS` in LIT.
>
>
> I just thought about reversing this. How about if you do something like:
>
>   // RUN: not %clang -v -fxray-instrument -c %s
>   // XFAIL: x86_64-, arm7-
>
>
> I suspect that would be sufficient to work as an `XPASS` of sorts?


Good idea, thanks. I've used it.


https://reviews.llvm.org/D24799



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

[PATCH] D24799: [XRay] Check in Clang whether XRay supports the target when -fxray-instrument is passed

[Serge Rogatch via cfe-commits]

rSerge updated this revision to Diff 73673.

https://reviews.llvm.org/D24799

Files:
  lib/Driver/Tools.cpp
  test/Driver/xray-instrument.c


Index: test/Driver/xray-instrument.c
===
--- test/Driver/xray-instrument.c
+++ test/Driver/xray-instrument.c
@@ -0,0 +1,3 @@
+// RUN: not %clang -v -fxray-instrument -c %s
+// XFAIL: amd64-, x86_64-, x86_64h-, arm
+typedef int a;
Index: lib/Driver/Tools.cpp
===
--- lib/Driver/Tools.cpp
+++ lib/Driver/Tools.cpp
@@ -4784,7 +4784,20 @@
 
   if (Args.hasFlag(options::OPT_fxray_instrument,
options::OPT_fnoxray_instrument, false)) {
-CmdArgs.push_back("-fxray-instrument");
+const char* const XRayInstrumentOption = "-fxray-instrument";
+switch(getToolChain().getArch()) {
+case llvm::Triple::arm:
+case llvm::Triple::x86_64:
+  break;
+default:
+{
+  std::string Feature(XRayInstrumentOption);
+  Feature += " on ";
+  Feature += Triple.getArchName().data();
+  D.Diag(diag::err_drv_clang_unsupported) << Feature;
+  break;
+} }
+CmdArgs.push_back(XRayInstrumentOption);
 if (const Arg *A =
 Args.getLastArg(options::OPT_fxray_instruction_threshold_,
 options::OPT_fxray_instruction_threshold_EQ)) {


Index: test/Driver/xray-instrument.c
===
--- test/Driver/xray-instrument.c
+++ test/Driver/xray-instrument.c
@@ -0,0 +1,3 @@
+// RUN: not %clang -v -fxray-instrument -c %s
+// XFAIL: amd64-, x86_64-, x86_64h-, arm
+typedef int a;
Index: lib/Driver/Tools.cpp
===
--- lib/Driver/Tools.cpp
+++ lib/Driver/Tools.cpp
@@ -4784,7 +4784,20 @@
 
   if (Args.hasFlag(options::OPT_fxray_instrument,
options::OPT_fnoxray_instrument, false)) {
-CmdArgs.push_back("-fxray-instrument");
+const char* const XRayInstrumentOption = "-fxray-instrument";
+switch(getToolChain().getArch()) {
+case llvm::Triple::arm:
+case llvm::Triple::x86_64:
+  break;
+default:
+{
+  std::string Feature(XRayInstrumentOption);
+  Feature += " on ";
+  Feature += Triple.getArchName().data();
+  D.Diag(diag::err_drv_clang_unsupported) << Feature;
+  break;
+} }
+CmdArgs.push_back(XRayInstrumentOption);
 if (const Arg *A =
 Args.getLastArg(options::OPT_fxray_instruction_threshold_,
 options::OPT_fxray_instruction_threshold_EQ)) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D24799: [XRay] Check in Clang whether XRay supports the target when -fxray-instrument is passed

[Serge Rogatch via cfe-commits]

rSerge added a comment.

In https://reviews.llvm.org/D24799#563058, @dberris wrote:

> Are we sure this will not fail on Windows? i.e. have you built/run the tests 
> on Windws ARM or X86_64?


The test itself passes for `arm-pc-win32` and `x86_64-pc-win32` on my machine. 
So in this sense it doesn't fail.
However, the feature of this patch doesn't cover the OS part of the triple: 
currently because of `mprotect` and other Linux-only code, XRay is only 
supported on Linux. However, clang (also with this patch) doesn't check for 
Linux target, and on non-Linux machines either LLVM backend will emit an error 
later (saying that XRay is not supported on Thumb - this happens because 
Windows is Thumb-only for ARM target), or I expect linker errors because code 
generation on Windows references some API of compiler-rt, which doesn't get 
compiled on Windows, even if it's x86_64.



> dberris wrote in Tools.cpp:4787
> Why can't this just be a `const string`, or a `const StringRef`?

Is there any advantage over `const char* const` here?

> dberris wrote in Tools.cpp:4796
> I'm not a fan of calling `.data()` here -- if this returns a `StringRef` I'd 
> much rather turn it into a string directly. Either that or you can even 
> string these together. Say something like:
> 
>   default:
> D.Diag(...) << (XRayInstrumentOption + " on " + 
> Triple.getArchName().str());
> break;

It returns `StringRef`. `.str()` would construct a `std::string`, which seems 
unnecessary.  Of course this piece is not performance-critical, but why not to 
just use `operator+=` taking as the argument `const char* const`?

> dberris wrote in Tools.cpp:4799
> As part of my submitting this change upstream, I had to format it accordingly 
> with `clang-format` -- you might want to do the same so that the formatting 
> is in-line with the LLVM developers style guidelines.

Done.

https://reviews.llvm.org/D24799



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

[PATCH] D24799: [XRay] Check in Clang whether XRay supports the target when -fxray-instrument is passed

[Serge Rogatch via cfe-commits]

rSerge updated this revision to Diff 73824.

https://reviews.llvm.org/D24799

Files:
  lib/Driver/Tools.cpp
  test/Driver/xray-instrument.c


Index: test/Driver/xray-instrument.c
===
--- test/Driver/xray-instrument.c
+++ test/Driver/xray-instrument.c
@@ -0,0 +1,3 @@
+// RUN: not %clang -v -fxray-instrument -c %s
+// XFAIL: amd64-, x86_64-, x86_64h-, arm
+typedef int a;
Index: lib/Driver/Tools.cpp
===
--- lib/Driver/Tools.cpp
+++ lib/Driver/Tools.cpp
@@ -4784,7 +4784,20 @@
 
   if (Args.hasFlag(options::OPT_fxray_instrument,
options::OPT_fnoxray_instrument, false)) {
-CmdArgs.push_back("-fxray-instrument");
+const char *const XRayInstrumentOption = "-fxray-instrument";
+switch (getToolChain().getArch()) {
+case llvm::Triple::arm:
+case llvm::Triple::x86_64:
+  break;
+default: {
+  std::string Feature(XRayInstrumentOption);
+  Feature += " on ";
+  Feature += Triple.getArchName().data();
+  D.Diag(diag::err_drv_clang_unsupported) << Feature;
+  break;
+}
+}
+CmdArgs.push_back(XRayInstrumentOption);
 if (const Arg *A =
 Args.getLastArg(options::OPT_fxray_instruction_threshold_,
 options::OPT_fxray_instruction_threshold_EQ)) {


Index: test/Driver/xray-instrument.c
===
--- test/Driver/xray-instrument.c
+++ test/Driver/xray-instrument.c
@@ -0,0 +1,3 @@
+// RUN: not %clang -v -fxray-instrument -c %s
+// XFAIL: amd64-, x86_64-, x86_64h-, arm
+typedef int a;
Index: lib/Driver/Tools.cpp
===
--- lib/Driver/Tools.cpp
+++ lib/Driver/Tools.cpp
@@ -4784,7 +4784,20 @@
 
   if (Args.hasFlag(options::OPT_fxray_instrument,
options::OPT_fnoxray_instrument, false)) {
-CmdArgs.push_back("-fxray-instrument");
+const char *const XRayInstrumentOption = "-fxray-instrument";
+switch (getToolChain().getArch()) {
+case llvm::Triple::arm:
+case llvm::Triple::x86_64:
+  break;
+default: {
+  std::string Feature(XRayInstrumentOption);
+  Feature += " on ";
+  Feature += Triple.getArchName().data();
+  D.Diag(diag::err_drv_clang_unsupported) << Feature;
+  break;
+}
+}
+CmdArgs.push_back(XRayInstrumentOption);
 if (const Arg *A =
 Args.getLastArg(options::OPT_fxray_instruction_threshold_,
 options::OPT_fxray_instruction_threshold_EQ)) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D24799: [XRay] Check in Clang whether XRay supports the target when -fxray-instrument is passed

[Serge Rogatch via cfe-commits]

rSerge added a comment.

I have extended this feature to check for OS support too (currently Linux 
only). I can't commit it so far because I don't know how to implement a test. 
XFAIL cannot check for both CPU and OS: it can only check for one of them. I 
tried to implement 2 tests instead like these:

1. ```// RUN: not %clang -v -fxray-instrument -c %s

// XFAIL: Linux
// REQUIRES-ANY: amd64-, x86_64, x86_64h, arm
typedef int a;

  2) ```// RUN: not %clang -v -fxray-instrument -c %s
  // XFAIL: amd64-, x86_64, x86_64h, arm
  // REQUIRES: Linux
  typedef int a;

However the problem with REQUIRES / REQUIRES-ANY is that they only check in LIT 
features, but not in the target triple. So everything becomes unsupported.

Does anyone have any ideas on how to implement the tests for Clang checking for 
both OS and CPU? I have 2 options in mind:

1. extend LIT, putting OS and CPU into the feature list
2. implement the test via GTest, rather than LIT




Comment at: lib/Driver/Tools.cpp:4796
+  Feature += " on ";
+  Feature += Triple.getArchName().data();
+  D.Diag(diag::err_drv_clang_unsupported) << Feature;

dberris wrote:
> rSerge wrote:
> > dberris wrote:
> > > I'm not a fan of calling `.data()` here -- if this returns a `StringRef` 
> > > I'd much rather turn it into a string directly. Either that or you can 
> > > even string these together. Say something like:
> > > 
> > > ```
> > > default:
> > >   D.Diag(...) << (XRayInstrumentOption + " on " + 
> > > Triple.getArchName().str());
> > >   break;
> > > ```
> > It returns `StringRef`. `.str()` would construct a `std::string`, which 
> > seems unnecessary.  Of course this piece is not performance-critical, but 
> > why not to just use `operator+=` taking as the argument `const char* const`?
> .data() doesn't necessarily have a null terminator.
> 
> Constructing a string out of successive +'s invoke move constructors on 
> std::string, which makes it as efficient if not more efficient than growing a 
> single string this way. At any rate it's much more readable if you did it in 
> a single line.
I see now, changing.


https://reviews.llvm.org/D24799



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

[PATCH] D24799: [XRay] Check in Clang whether XRay supports the target when -fxray-instrument is passed

[Serge Rogatch via cfe-commits]

rSerge updated this revision to Diff 74829.

https://reviews.llvm.org/D24799

Files:
  lib/Driver/Tools.cpp


Index: lib/Driver/Tools.cpp
===
--- lib/Driver/Tools.cpp
+++ lib/Driver/Tools.cpp
@@ -4804,7 +4804,16 @@
 
   if (Args.hasFlag(options::OPT_fxray_instrument,
options::OPT_fnoxray_instrument, false)) {
-CmdArgs.push_back("-fxray-instrument");
+const std::string XRayInstrumentOption("-fxray-instrument");
+if (Triple.getOS() == llvm::Triple::Linux &&
+(Triple.getArch() == llvm::Triple::arm ||
+ Triple.getArch() == llvm::Triple::x86_64)) {
+  // Supported.
+} else {
+  D.Diag(diag::err_drv_clang_unsupported) << (XRayInstrumentOption + " on "
++ Triple.str());
+}
+CmdArgs.push_back(XRayInstrumentOption.c_str());
 if (const Arg *A =
 Args.getLastArg(options::OPT_fxray_instruction_threshold_,
 options::OPT_fxray_instruction_threshold_EQ)) {


Index: lib/Driver/Tools.cpp
===
--- lib/Driver/Tools.cpp
+++ lib/Driver/Tools.cpp
@@ -4804,7 +4804,16 @@
 
   if (Args.hasFlag(options::OPT_fxray_instrument,
options::OPT_fnoxray_instrument, false)) {
-CmdArgs.push_back("-fxray-instrument");
+const std::string XRayInstrumentOption("-fxray-instrument");
+if (Triple.getOS() == llvm::Triple::Linux &&
+(Triple.getArch() == llvm::Triple::arm ||
+ Triple.getArch() == llvm::Triple::x86_64)) {
+  // Supported.
+} else {
+  D.Diag(diag::err_drv_clang_unsupported) << (XRayInstrumentOption + " on "
++ Triple.str());
+}
+CmdArgs.push_back(XRayInstrumentOption.c_str());
 if (const Arg *A =
 Args.getLastArg(options::OPT_fxray_instruction_threshold_,
 options::OPT_fxray_instruction_threshold_EQ)) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D24799: [XRay] Check in Clang whether XRay supports the target when -fxray-instrument is passed

[Serge Rogatch via cfe-commits]

rSerge added a comment.

Ping? (please, see the question about the test)


https://reviews.llvm.org/D24799



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

[PATCH] D23932: [XRay] ARM 32-bit no-Thumb support in Clang

[Serge Rogatch via cfe-commits]

rSerge created this revision.
rSerge added reviewers: dberris, rengolin.
rSerge added a subscriber: cfe-commits.
Herald added subscribers: dberris, samparker, rengolin, aemerson.

Just a test for now, adapted from x86_64 tests of XRay.

https://reviews.llvm.org/D23932

Files:
  test/CodeGen/xray-attributes-supported-arm.cpp

Index: test/CodeGen/xray-attributes-supported-arm.cpp
===
--- test/CodeGen/xray-attributes-supported-arm.cpp
+++ test/CodeGen/xray-attributes-supported-arm.cpp
@@ -0,0 +1,13 @@
+// RUN: %clang_cc1 %s -fxray-instrument -std=c++11 -x c++ -emit-llvm -o - 
-triple arm-unknown-linux-gnu | FileCheck %s
+
+// Make sure that the LLVM attribute for XRay-annotated functions do show up.
+[[clang::xray_always_instrument]] void foo() {
+// CHECK: define void @_Z3foov() #0
+};
+
+[[clang::xray_never_instrument]] void bar() {
+// CHECK: define void @_Z3barv() #1
+};
+
+// CHECK: #0 = {{.*}}"function-instrument"="xray-always"
+// CHECK: #1 = {{.*}}"function-instrument"="xray-never"


Index: test/CodeGen/xray-attributes-supported-arm.cpp
===
--- test/CodeGen/xray-attributes-supported-arm.cpp
+++ test/CodeGen/xray-attributes-supported-arm.cpp
@@ -0,0 +1,13 @@
+// RUN: %clang_cc1 %s -fxray-instrument -std=c++11 -x c++ -emit-llvm -o - -triple arm-unknown-linux-gnu | FileCheck %s
+
+// Make sure that the LLVM attribute for XRay-annotated functions do show up.
+[[clang::xray_always_instrument]] void foo() {
+// CHECK: define void @_Z3foov() #0
+};
+
+[[clang::xray_never_instrument]] void bar() {
+// CHECK: define void @_Z3barv() #1
+};
+
+// CHECK: #0 = {{.*}}"function-instrument"="xray-always"
+// CHECK: #1 = {{.*}}"function-instrument"="xray-never"
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D23932: [XRay] ARM 32-bit no-Thumb support in Clang

[Serge Rogatch via cfe-commits]

rSerge added a comment.

I don't have commit access rights. Could someone commit?


https://reviews.llvm.org/D23932



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

Re: [PATCH] D23932: [XRay] ARM 32-bit no-Thumb support in Clang

[Serge Rogatch via cfe-commits]

rSerge updated this revision to Diff 71629.

https://reviews.llvm.org/D23932

Files:
  test/CodeGen/xray-attributes-supported-arm.cpp

Index: test/CodeGen/xray-attributes-supported-arm.cpp
===
--- test/CodeGen/xray-attributes-supported-arm.cpp
+++ test/CodeGen/xray-attributes-supported-arm.cpp
@@ -0,0 +1,13 @@
+// RUN: %clang_cc1 %s -fxray-instrument -std=c++11 -x c++ -emit-llvm -o - 
-triple arm-unknown-linux-gnu | FileCheck %s
+
+// Make sure that the LLVM attribute for XRay-annotated functions do show up.
+[[clang::xray_always_instrument]] void foo() {
+// CHECK: define void @_Z3foov() #0
+};
+
+[[clang::xray_never_instrument]] void bar() {
+// CHECK: define void @_Z3barv() #1
+};
+
+// CHECK: #0 = {{.*}}"function-instrument"="xray-always"
+// CHECK: #1 = {{.*}}"function-instrument"="xray-never"


Index: test/CodeGen/xray-attributes-supported-arm.cpp
===
--- test/CodeGen/xray-attributes-supported-arm.cpp
+++ test/CodeGen/xray-attributes-supported-arm.cpp
@@ -0,0 +1,13 @@
+// RUN: %clang_cc1 %s -fxray-instrument -std=c++11 -x c++ -emit-llvm -o - -triple arm-unknown-linux-gnu | FileCheck %s
+
+// Make sure that the LLVM attribute for XRay-annotated functions do show up.
+[[clang::xray_always_instrument]] void foo() {
+// CHECK: define void @_Z3foov() #0
+};
+
+[[clang::xray_never_instrument]] void bar() {
+// CHECK: define void @_Z3barv() #1
+};
+
+// CHECK: #0 = {{.*}}"function-instrument"="xray-always"
+// CHECK: #1 = {{.*}}"function-instrument"="xray-never"
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D23932: [XRay] ARM 32-bit no-Thumb support in Clang

[Serge Rogatch via cfe-commits]

rSerge updated this revision to Diff 71635.
rSerge added a comment.

Fixed patch file format.


https://reviews.llvm.org/D23932

Files:
  test/CodeGen/xray-attributes-supported-arm.cpp

Index: test/CodeGen/xray-attributes-supported-arm.cpp
===
--- test/CodeGen/xray-attributes-supported-arm.cpp
+++ test/CodeGen/xray-attributes-supported-arm.cpp
@@ -0,0 +1,13 @@
+// RUN: %clang_cc1 %s -fxray-instrument -std=c++11 -x c++ -emit-llvm -o - 
-triple arm-unknown-linux-gnu | FileCheck %s
+
+// Make sure that the LLVM attribute for XRay-annotated functions do show up.
+[[clang::xray_always_instrument]] void foo() {
+// CHECK: define void @_Z3foov() #0
+};
+
+[[clang::xray_never_instrument]] void bar() {
+// CHECK: define void @_Z3barv() #1
+};
+
+// CHECK: #0 = {{.*}}"function-instrument"="xray-always"
+// CHECK: #1 = {{.*}}"function-instrument"="xray-never"


Index: test/CodeGen/xray-attributes-supported-arm.cpp
===
--- test/CodeGen/xray-attributes-supported-arm.cpp
+++ test/CodeGen/xray-attributes-supported-arm.cpp
@@ -0,0 +1,13 @@
+// RUN: %clang_cc1 %s -fxray-instrument -std=c++11 -x c++ -emit-llvm -o - -triple arm-unknown-linux-gnu | FileCheck %s
+
+// Make sure that the LLVM attribute for XRay-annotated functions do show up.
+[[clang::xray_always_instrument]] void foo() {
+// CHECK: define void @_Z3foov() #0
+};
+
+[[clang::xray_never_instrument]] void bar() {
+// CHECK: define void @_Z3barv() #1
+};
+
+// CHECK: #0 = {{.*}}"function-instrument"="xray-always"
+// CHECK: #1 = {{.*}}"function-instrument"="xray-never"
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D26415: [XRay] Support AArch64 in Clang

[Serge Rogatch via cfe-commits]

rSerge updated this revision to Diff 78216.
rSerge added a comment.

Rebased to the latest version of LLVM sources.


https://reviews.llvm.org/D26415

Files:
  lib/Driver/Tools.cpp
  test/Driver/XRay/xray-instrument-cpu.c
  test/Driver/XRay/xray-instrument-os.c


Index: test/Driver/XRay/xray-instrument-os.c
===
--- test/Driver/XRay/xray-instrument-os.c
+++ test/Driver/XRay/xray-instrument-os.c
@@ -1,4 +1,4 @@
 // RUN: not %clang -o /dev/null -v -fxray-instrument -c %s
 // XFAIL: -linux-
-// REQUIRES-ANY: amd64, x86_64, x86_64h, arm
+// REQUIRES-ANY: amd64, x86_64, x86_64h, arm, aarch64, arm64
 typedef int a;
Index: test/Driver/XRay/xray-instrument-cpu.c
===
--- test/Driver/XRay/xray-instrument-cpu.c
+++ test/Driver/XRay/xray-instrument-cpu.c
@@ -1,4 +1,4 @@
 // RUN: not %clang -o /dev/null -v -fxray-instrument -c %s
-// XFAIL: amd64-, x86_64-, x86_64h-, arm
+// XFAIL: amd64-, x86_64-, x86_64h-, arm, aarch64, arm64
 // REQUIRES: linux
 typedef int a;
Index: lib/Driver/Tools.cpp
===
--- lib/Driver/Tools.cpp
+++ lib/Driver/Tools.cpp
@@ -4900,14 +4900,20 @@
   if (Args.hasFlag(options::OPT_fxray_instrument,
options::OPT_fnoxray_instrument, false)) {
 const char *const XRayInstrumentOption = "-fxray-instrument";
-if (Triple.getOS() == llvm::Triple::Linux &&
-(Triple.getArch() == llvm::Triple::arm ||
- Triple.getArch() == llvm::Triple::x86_64)) {
-  // Supported.
-} else {
+if (Triple.getOS() == llvm::Triple::Linux)
+  switch (Triple.getArch()) {
+  case llvm::Triple::x86_64:
+  case llvm::Triple::arm:
+  case llvm::Triple::aarch64:
+// Supported.
+break;
+  default:
+D.Diag(diag::err_drv_clang_unsupported)
+<< (std::string(XRayInstrumentOption) + " on " + Triple.str());
+  }
+else
   D.Diag(diag::err_drv_clang_unsupported)
-  << (std::string(XRayInstrumentOption) + " on " + Triple.str());
-}
+  << (std::string(XRayInstrumentOption) + " on non-Linux target OS");
 CmdArgs.push_back(XRayInstrumentOption);
 if (const Arg *A =
 Args.getLastArg(options::OPT_fxray_instruction_threshold_,


Index: test/Driver/XRay/xray-instrument-os.c
===
--- test/Driver/XRay/xray-instrument-os.c
+++ test/Driver/XRay/xray-instrument-os.c
@@ -1,4 +1,4 @@
 // RUN: not %clang -o /dev/null -v -fxray-instrument -c %s
 // XFAIL: -linux-
-// REQUIRES-ANY: amd64, x86_64, x86_64h, arm
+// REQUIRES-ANY: amd64, x86_64, x86_64h, arm, aarch64, arm64
 typedef int a;
Index: test/Driver/XRay/xray-instrument-cpu.c
===
--- test/Driver/XRay/xray-instrument-cpu.c
+++ test/Driver/XRay/xray-instrument-cpu.c
@@ -1,4 +1,4 @@
 // RUN: not %clang -o /dev/null -v -fxray-instrument -c %s
-// XFAIL: amd64-, x86_64-, x86_64h-, arm
+// XFAIL: amd64-, x86_64-, x86_64h-, arm, aarch64, arm64
 // REQUIRES: linux
 typedef int a;
Index: lib/Driver/Tools.cpp
===
--- lib/Driver/Tools.cpp
+++ lib/Driver/Tools.cpp
@@ -4900,14 +4900,20 @@
   if (Args.hasFlag(options::OPT_fxray_instrument,
options::OPT_fnoxray_instrument, false)) {
 const char *const XRayInstrumentOption = "-fxray-instrument";
-if (Triple.getOS() == llvm::Triple::Linux &&
-(Triple.getArch() == llvm::Triple::arm ||
- Triple.getArch() == llvm::Triple::x86_64)) {
-  // Supported.
-} else {
+if (Triple.getOS() == llvm::Triple::Linux)
+  switch (Triple.getArch()) {
+  case llvm::Triple::x86_64:
+  case llvm::Triple::arm:
+  case llvm::Triple::aarch64:
+// Supported.
+break;
+  default:
+D.Diag(diag::err_drv_clang_unsupported)
+<< (std::string(XRayInstrumentOption) + " on " + Triple.str());
+  }
+else
   D.Diag(diag::err_drv_clang_unsupported)
-  << (std::string(XRayInstrumentOption) + " on " + Triple.str());
-}
+  << (std::string(XRayInstrumentOption) + " on non-Linux target OS");
 CmdArgs.push_back(XRayInstrumentOption);
 if (const Arg *A =
 Args.getLastArg(options::OPT_fxray_instruction_threshold_,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D24799: [XRay] Check in Clang whether XRay supports the target when -fxray-instrument is passed

[Serge Rogatch via cfe-commits]

rSerge added a comment.

@dberris , below is the list of features supported by `REQUIRES` and 
`REQUIRES-ANY` for me:

  set(['system-windows', 'nozlib', u'amdgpu-registered-target', 
'case-insensitive-filesystem', u'msp430-registered-target', 
u'sparc-registered-target', 'libgcc', 'staticanalyzer', 'native', 
u'powerpc-registered-target', 'non-ps4-sdk', 'clang-driver', 'backtrace', 
'crash-recovery', u'bpf-registered-target', u'hexagon-registered-target', 
'not_ubsan', u'x86-registered-target', u'aarch64-registered-target', 
u'arm-registered-target', 'not_asan', u'nvptx-registered-target', 
u'mips-registered-target', u'systemz-registered-target', 
u'lanai-registered-target', u'xcore-registered-target'])

So there is `system-windows`, however, there is no target CPU among the 
features. I understand that the features with `-registered-target` suffixes 
mean the targets supported with the current Clang build.
I am not submitting the tests because they do not work for the above reason. I 
also had to remove the old test because the new code checks the OS, but not 
only the CPU.


https://reviews.llvm.org/D24799



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

[PATCH] D24799: [XRay] Check in Clang whether XRay supports the target when -fxray-instrument is passed

[Serge Rogatch via cfe-commits]

rSerge updated this revision to Diff 75452.
rSerge added a comment.

I had to add the root directories `a` and `b` manually, as I couldn't find an 
`svn diff` argument for that.
The code file `Tools.cpp` was run via `clang-format`, then just my changes were 
copy-pasted.
2 tests have been added to enable both OS and CPU checking.
`const string` approach was leading to undefined behavior, because the lifetime 
of the string is shorter than the lifetime of the `const char *` array using 
its data. So I had to return to `const char* const`.


https://reviews.llvm.org/D24799

Files:
  lib/Driver/Tools.cpp
  test/Driver/XRay/lit.local.cfg
  test/Driver/XRay/xray-instrument-cpu.c
  test/Driver/XRay/xray-instrument-os.c


Index: test/Driver/XRay/xray-instrument-os.c
===
--- test/Driver/XRay/xray-instrument-os.c
+++ test/Driver/XRay/xray-instrument-os.c
@@ -0,0 +1,4 @@
+// RUN: not %clang -v -fxray-instrument -c %s
+// XFAIL: -linux-
+// REQUIRES-ANY: amd64, x86_64, x86_64h, arm
+typedef int a;
Index: test/Driver/XRay/xray-instrument-cpu.c
===
--- test/Driver/XRay/xray-instrument-cpu.c
+++ test/Driver/XRay/xray-instrument-cpu.c
@@ -0,0 +1,4 @@
+// RUN: not %clang -v -fxray-instrument -c %s
+// XFAIL: amd64-, x86_64-, x86_64h-, arm
+// REQUIRES: linux
+typedef int a;
Index: test/Driver/XRay/lit.local.cfg
===
--- test/Driver/XRay/lit.local.cfg
+++ test/Driver/XRay/lit.local.cfg
@@ -0,0 +1,2 @@
+target_triple_components = config.target_triple.split('-')
+config.available_features.update(target_triple_components)
Index: lib/Driver/Tools.cpp
===
--- lib/Driver/Tools.cpp
+++ lib/Driver/Tools.cpp
@@ -4804,7 +4804,16 @@
 
   if (Args.hasFlag(options::OPT_fxray_instrument,
options::OPT_fnoxray_instrument, false)) {
-CmdArgs.push_back("-fxray-instrument");
+const char *const XRayInstrumentOption = "-fxray-instrument";
+if (Triple.getOS() == llvm::Triple::Linux &&
+(Triple.getArch() == llvm::Triple::arm ||
+ Triple.getArch() == llvm::Triple::x86_64)) {
+  // Supported.
+} else {
+  D.Diag(diag::err_drv_clang_unsupported)
+  << (std::string(XRayInstrumentOption) + " on " + Triple.str());
+}
+CmdArgs.push_back(XRayInstrumentOption);
 if (const Arg *A =
 Args.getLastArg(options::OPT_fxray_instruction_threshold_,
 options::OPT_fxray_instruction_threshold_EQ)) {


Index: test/Driver/XRay/xray-instrument-os.c
===
--- test/Driver/XRay/xray-instrument-os.c
+++ test/Driver/XRay/xray-instrument-os.c
@@ -0,0 +1,4 @@
+// RUN: not %clang -v -fxray-instrument -c %s
+// XFAIL: -linux-
+// REQUIRES-ANY: amd64, x86_64, x86_64h, arm
+typedef int a;
Index: test/Driver/XRay/xray-instrument-cpu.c
===
--- test/Driver/XRay/xray-instrument-cpu.c
+++ test/Driver/XRay/xray-instrument-cpu.c
@@ -0,0 +1,4 @@
+// RUN: not %clang -v -fxray-instrument -c %s
+// XFAIL: amd64-, x86_64-, x86_64h-, arm
+// REQUIRES: linux
+typedef int a;
Index: test/Driver/XRay/lit.local.cfg
===
--- test/Driver/XRay/lit.local.cfg
+++ test/Driver/XRay/lit.local.cfg
@@ -0,0 +1,2 @@
+target_triple_components = config.target_triple.split('-')
+config.available_features.update(target_triple_components)
Index: lib/Driver/Tools.cpp
===
--- lib/Driver/Tools.cpp
+++ lib/Driver/Tools.cpp
@@ -4804,7 +4804,16 @@
 
   if (Args.hasFlag(options::OPT_fxray_instrument,
options::OPT_fnoxray_instrument, false)) {
-CmdArgs.push_back("-fxray-instrument");
+const char *const XRayInstrumentOption = "-fxray-instrument";
+if (Triple.getOS() == llvm::Triple::Linux &&
+(Triple.getArch() == llvm::Triple::arm ||
+ Triple.getArch() == llvm::Triple::x86_64)) {
+  // Supported.
+} else {
+  D.Diag(diag::err_drv_clang_unsupported)
+  << (std::string(XRayInstrumentOption) + " on " + Triple.str());
+}
+CmdArgs.push_back(XRayInstrumentOption);
 if (const Arg *A =
 Args.getLastArg(options::OPT_fxray_instruction_threshold_,
 options::OPT_fxray_instruction_threshold_EQ)) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D24799: [XRay] Check in Clang whether XRay supports the target when -fxray-instrument is passed

[Serge Rogatch via cfe-commits]

rSerge added a comment.

@dberris , thanks! I ran clang-format on Tools.cpp , then just copy-pasted the 
changed piece. Do you remember, was the whitespace problem in this file? Or was 
it in the tests?


Repository:
  rL LLVM

https://reviews.llvm.org/D24799



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

[PATCH] D24799: [XRay] Check in Clang whether XRay supports the target when -fxray-instrument is passed

[Serge Rogatch via cfe-commits]

rSerge added a comment.

@dberris , I've checked the .diff file which I was uploading to Phabricator, 
and it doesn't seem to contain trailing whitespace. Please, see the screenshot:
F2536547: Whitespace.jpg 
So it seems that Phabricator or your downloading tool adds the whitespace.
The only potential problem is in "CR,LF" line terminator, while you may be 
using Linux "LF" only.


Repository:
  rL LLVM

https://reviews.llvm.org/D24799



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

[PATCH] D26415: [XRay] Support AArch64 in Clang

[Serge Rogatch via cfe-commits]

rSerge created this revision.
rSerge added reviewers: dberris, rengolin.
rSerge added subscribers: iid_iunknown, cfe-commits.
Herald added a subscriber: aemerson.

This patch adds XRay support in Clang for AArch64 target.


https://reviews.llvm.org/D26415

Files:
  lib/Driver/Tools.cpp
  test/Driver/XRay/xray-instrument-cpu.c
  test/Driver/XRay/xray-instrument-os.c


Index: test/Driver/XRay/xray-instrument-os.c
===
--- test/Driver/XRay/xray-instrument-os.c
+++ test/Driver/XRay/xray-instrument-os.c
@@ -1,4 +1,4 @@
 // RUN: not %clang -o /dev/null -v -fxray-instrument -c %s
 // XFAIL: -linux-
-// REQUIRES-ANY: amd64, x86_64, x86_64h, arm
+// REQUIRES-ANY: amd64, x86_64, x86_64h, arm, aarch64, arm64
 typedef int a;
Index: test/Driver/XRay/xray-instrument-cpu.c
===
--- test/Driver/XRay/xray-instrument-cpu.c
+++ test/Driver/XRay/xray-instrument-cpu.c
@@ -1,4 +1,4 @@
 // RUN: not %clang -o /dev/null -v -fxray-instrument -c %s
-// XFAIL: amd64-, x86_64-, x86_64h-, arm
+// XFAIL: amd64-, x86_64-, x86_64h-, arm, aarch64, arm64
 // REQUIRES: linux
 typedef int a;
Index: lib/Driver/Tools.cpp
===
--- lib/Driver/Tools.cpp
+++ lib/Driver/Tools.cpp
@@ -4902,7 +4902,8 @@
 const char *const XRayInstrumentOption = "-fxray-instrument";
 if (Triple.getOS() == llvm::Triple::Linux &&
 (Triple.getArch() == llvm::Triple::arm ||
- Triple.getArch() == llvm::Triple::x86_64)) {
+ Triple.getArch() == llvm::Triple::x86_64 ||
+ Triple.getArch() == llvm::Triple::aarch64)) {
   // Supported.
 } else {
   D.Diag(diag::err_drv_clang_unsupported)


Index: test/Driver/XRay/xray-instrument-os.c
===
--- test/Driver/XRay/xray-instrument-os.c
+++ test/Driver/XRay/xray-instrument-os.c
@@ -1,4 +1,4 @@
 // RUN: not %clang -o /dev/null -v -fxray-instrument -c %s
 // XFAIL: -linux-
-// REQUIRES-ANY: amd64, x86_64, x86_64h, arm
+// REQUIRES-ANY: amd64, x86_64, x86_64h, arm, aarch64, arm64
 typedef int a;
Index: test/Driver/XRay/xray-instrument-cpu.c
===
--- test/Driver/XRay/xray-instrument-cpu.c
+++ test/Driver/XRay/xray-instrument-cpu.c
@@ -1,4 +1,4 @@
 // RUN: not %clang -o /dev/null -v -fxray-instrument -c %s
-// XFAIL: amd64-, x86_64-, x86_64h-, arm
+// XFAIL: amd64-, x86_64-, x86_64h-, arm, aarch64, arm64
 // REQUIRES: linux
 typedef int a;
Index: lib/Driver/Tools.cpp
===
--- lib/Driver/Tools.cpp
+++ lib/Driver/Tools.cpp
@@ -4902,7 +4902,8 @@
 const char *const XRayInstrumentOption = "-fxray-instrument";
 if (Triple.getOS() == llvm::Triple::Linux &&
 (Triple.getArch() == llvm::Triple::arm ||
- Triple.getArch() == llvm::Triple::x86_64)) {
+ Triple.getArch() == llvm::Triple::x86_64 ||
+ Triple.getArch() == llvm::Triple::aarch64)) {
   // Supported.
 } else {
   D.Diag(diag::err_drv_clang_unsupported)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D26415: [XRay] Support AArch64 in Clang

[Serge Rogatch via cfe-commits]

rSerge added inline comments.



Comment at: lib/Driver/Tools.cpp:4903-4906
 if (Triple.getOS() == llvm::Triple::Linux &&
 (Triple.getArch() == llvm::Triple::arm ||
- Triple.getArch() == llvm::Triple::x86_64)) {
+ Triple.getArch() == llvm::Triple::x86_64 ||
+ Triple.getArch() == llvm::Triple::aarch64)) {

dberris wrote:
> I'm wondering whether it's worth turning this into a `switch` statement now 
> that we have more than two supported architectures?
I think that would lead to more awkward code: there wouldn't be a single 
decision outcome point (like the current `else` block), so to avoid duplicating 
the code which currently prints the message, a `bool` variable would be needed. 
I think it's more neat to just enumerate all the OS&CPU combinations in the 
`if` condition for now.
This place is not performance-critical and the compiler should convert 
appropriate `if`s into `switch`es anyway.


https://reviews.llvm.org/D26415



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

[PATCH] D26415: [XRay] Support AArch64 in Clang

[Serge Rogatch via cfe-commits]

rSerge added inline comments.



Comment at: lib/Driver/Tools.cpp:4903-4906
 if (Triple.getOS() == llvm::Triple::Linux &&
 (Triple.getArch() == llvm::Triple::arm ||
- Triple.getArch() == llvm::Triple::x86_64)) {
+ Triple.getArch() == llvm::Triple::x86_64 ||
+ Triple.getArch() == llvm::Triple::aarch64)) {

dberris wrote:
> rSerge wrote:
> > dberris wrote:
> > > I'm wondering whether it's worth turning this into a `switch` statement 
> > > now that we have more than two supported architectures?
> > I think that would lead to more awkward code: there wouldn't be a single 
> > decision outcome point (like the current `else` block), so to avoid 
> > duplicating the code which currently prints the message, a `bool` variable 
> > would be needed. I think it's more neat to just enumerate all the OS&CPU 
> > combinations in the `if` condition for now.
> > This place is not performance-critical and the compiler should convert 
> > appropriate `if`s into `switch`es anyway.
> This is an issue of making it more readable, something like:
> 
> ```
> if (Triple.getOS() != llvm::Triple::Linux)
>   D.Diag(...) << ...; // Unsupported OS.
> switch (Triple.getArch()) {
>   case llvm::Triple::arm:
>   case llvm::Triple::x86_64:
>   case llvm::Tripe::aarch64:
> // Supported.
> break;
>   default:
> D.Diag(...) << ...;
> }
> ```
> 
> This way any new architectures become supported, they just get added to the 
> list of cases that short-circuit.
We can't say that an OS is supported or unsupported unless all CPU 
architectures for this OS support or don't support XRay, and this is not going 
to happen in the near future. So it is more accurate to say about the triple: 
some triples are supported and some are not. So in coding it is natural to 
check for the triple with `||` and `&&`.


https://reviews.llvm.org/D26415



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

[PATCH] D26415: [XRay] Support AArch64 in Clang

[Serge Rogatch via cfe-commits]

rSerge updated this revision to Diff 77537.

https://reviews.llvm.org/D26415

Files:
  lib/Driver/Tools.cpp
  test/Driver/XRay/xray-instrument-cpu.c
  test/Driver/XRay/xray-instrument-os.c


Index: test/Driver/XRay/xray-instrument-os.c
===
--- test/Driver/XRay/xray-instrument-os.c
+++ test/Driver/XRay/xray-instrument-os.c
@@ -1,4 +1,4 @@
 // RUN: not %clang -o /dev/null -v -fxray-instrument -c %s
 // XFAIL: -linux-
-// REQUIRES-ANY: amd64, x86_64, x86_64h, arm
+// REQUIRES-ANY: amd64, x86_64, x86_64h, arm, aarch64, arm64
 typedef int a;
Index: test/Driver/XRay/xray-instrument-cpu.c
===
--- test/Driver/XRay/xray-instrument-cpu.c
+++ test/Driver/XRay/xray-instrument-cpu.c
@@ -1,4 +1,4 @@
 // RUN: not %clang -o /dev/null -v -fxray-instrument -c %s
-// XFAIL: amd64-, x86_64-, x86_64h-, arm
+// XFAIL: amd64-, x86_64-, x86_64h-, arm, aarch64, arm64
 // REQUIRES: linux
 typedef int a;
Index: lib/Driver/Tools.cpp
===
--- lib/Driver/Tools.cpp
+++ lib/Driver/Tools.cpp
@@ -4900,11 +4900,16 @@
   if (Args.hasFlag(options::OPT_fxray_instrument,
options::OPT_fnoxray_instrument, false)) {
 const char *const XRayInstrumentOption = "-fxray-instrument";
-if (Triple.getOS() == llvm::Triple::Linux &&
-(Triple.getArch() == llvm::Triple::arm ||
- Triple.getArch() == llvm::Triple::x86_64)) {
+if (Triple.getOS() != llvm::Triple::Linux)
+  D.Diag(diag::err_drv_clang_unsupported)
+  << (std::string(XRayInstrumentOption) + " on non-Linux target OS.");
+switch (Triple.getArch()) {
+case llvm::Triple::x86_64:
+case llvm::Triple::arm:
+case llvm::Triple::aarch64:
   // Supported.
-} else {
+  break;
+default:
   D.Diag(diag::err_drv_clang_unsupported)
   << (std::string(XRayInstrumentOption) + " on " + Triple.str());
 }


Index: test/Driver/XRay/xray-instrument-os.c
===
--- test/Driver/XRay/xray-instrument-os.c
+++ test/Driver/XRay/xray-instrument-os.c
@@ -1,4 +1,4 @@
 // RUN: not %clang -o /dev/null -v -fxray-instrument -c %s
 // XFAIL: -linux-
-// REQUIRES-ANY: amd64, x86_64, x86_64h, arm
+// REQUIRES-ANY: amd64, x86_64, x86_64h, arm, aarch64, arm64
 typedef int a;
Index: test/Driver/XRay/xray-instrument-cpu.c
===
--- test/Driver/XRay/xray-instrument-cpu.c
+++ test/Driver/XRay/xray-instrument-cpu.c
@@ -1,4 +1,4 @@
 // RUN: not %clang -o /dev/null -v -fxray-instrument -c %s
-// XFAIL: amd64-, x86_64-, x86_64h-, arm
+// XFAIL: amd64-, x86_64-, x86_64h-, arm, aarch64, arm64
 // REQUIRES: linux
 typedef int a;
Index: lib/Driver/Tools.cpp
===
--- lib/Driver/Tools.cpp
+++ lib/Driver/Tools.cpp
@@ -4900,11 +4900,16 @@
   if (Args.hasFlag(options::OPT_fxray_instrument,
options::OPT_fnoxray_instrument, false)) {
 const char *const XRayInstrumentOption = "-fxray-instrument";
-if (Triple.getOS() == llvm::Triple::Linux &&
-(Triple.getArch() == llvm::Triple::arm ||
- Triple.getArch() == llvm::Triple::x86_64)) {
+if (Triple.getOS() != llvm::Triple::Linux)
+  D.Diag(diag::err_drv_clang_unsupported)
+  << (std::string(XRayInstrumentOption) + " on non-Linux target OS.");
+switch (Triple.getArch()) {
+case llvm::Triple::x86_64:
+case llvm::Triple::arm:
+case llvm::Triple::aarch64:
   // Supported.
-} else {
+  break;
+default:
   D.Diag(diag::err_drv_clang_unsupported)
   << (std::string(XRayInstrumentOption) + " on " + Triple.str());
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D26415: [XRay] Support AArch64 in Clang

[Serge Rogatch via cfe-commits]

rSerge marked 5 inline comments as done.
rSerge added inline comments.



Comment at: lib/Driver/Tools.cpp:4903-4906
 if (Triple.getOS() == llvm::Triple::Linux &&
 (Triple.getArch() == llvm::Triple::arm ||
- Triple.getArch() == llvm::Triple::x86_64)) {
+ Triple.getArch() == llvm::Triple::x86_64 ||
+ Triple.getArch() == llvm::Triple::aarch64)) {

dberris wrote:
> rSerge wrote:
> > dberris wrote:
> > > rSerge wrote:
> > > > dberris wrote:
> > > > > I'm wondering whether it's worth turning this into a `switch` 
> > > > > statement now that we have more than two supported architectures?
> > > > I think that would lead to more awkward code: there wouldn't be a 
> > > > single decision outcome point (like the current `else` block), so to 
> > > > avoid duplicating the code which currently prints the message, a `bool` 
> > > > variable would be needed. I think it's more neat to just enumerate all 
> > > > the OS&CPU combinations in the `if` condition for now.
> > > > This place is not performance-critical and the compiler should convert 
> > > > appropriate `if`s into `switch`es anyway.
> > > This is an issue of making it more readable, something like:
> > > 
> > > ```
> > > if (Triple.getOS() != llvm::Triple::Linux)
> > >   D.Diag(...) << ...; // Unsupported OS.
> > > switch (Triple.getArch()) {
> > >   case llvm::Triple::arm:
> > >   case llvm::Triple::x86_64:
> > >   case llvm::Tripe::aarch64:
> > > // Supported.
> > > break;
> > >   default:
> > > D.Diag(...) << ...;
> > > }
> > > ```
> > > 
> > > This way any new architectures become supported, they just get added to 
> > > the list of cases that short-circuit.
> > We can't say that an OS is supported or unsupported unless all CPU 
> > architectures for this OS support or don't support XRay, and this is not 
> > going to happen in the near future. So it is more accurate to say about the 
> > triple: some triples are supported and some are not. So in coding it is 
> > natural to check for the triple with `||` and `&&`.
> > We can't say that an OS is supported or unsupported unless all CPU 
> > architectures for this OS support or don't support XRay, and this is not 
> > going to happen in the near future.
> 
> I don't get it. Right now, as written, it doesn't matter what OS it is -- any 
> OS other than Linux wouldn't be supported anyway. Maybe I mis-wrote, but:
> 
> ```
> if (Triple.getOS() != llvm::Triple::Linux)
>   D.Diag(...) << ...;
> else switch(Triple.getArch()) {
>   ...
>   default:
> D.Diag(...) << ...;
> }
> ```
> 
> Is a direct translation that's more readable than the current complex if 
> statement conditional.
> 
> > So it is more accurate to say about the triple: some triples are supported 
> > and some are not. So in coding it is natural to check for the triple with 
> > || and &&.
> 
> Sure, but conditional is already unwieldy with just three supported platforms.
Changed.


https://reviews.llvm.org/D26415



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

[PATCH] D26415: [XRay] Support AArch64 in Clang

[Serge Rogatch via cfe-commits]

rSerge marked an inline comment as done.
rSerge added inline comments.



Comment at: lib/Driver/Tools.cpp:4903-4906
+if (Triple.getOS() != llvm::Triple::Linux)
+  D.Diag(diag::err_drv_clang_unsupported)
+  << (std::string(XRayInstrumentOption) + " on non-Linux target OS.");
+switch (Triple.getArch()) {

dberris wrote:
> Did you need to put an `else` before the switch? I don't know what happens 
> when you're on Windows non-supported platform -- will this cause two 
> diagnostics to be printed?
I thought that the statement after `D.Diag(diag::err_drv_clang_unsupported)` 
will not be executed. Let me check...


https://reviews.llvm.org/D26415



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

[PATCH] D26415: [XRay] Support AArch64 in Clang

[Serge Rogatch via cfe-commits]

rSerge updated this revision to Diff 77675.

https://reviews.llvm.org/D26415

Files:
  lib/Driver/Tools.cpp
  test/Driver/XRay/xray-instrument-cpu.c
  test/Driver/XRay/xray-instrument-os.c


Index: test/Driver/XRay/xray-instrument-os.c
===
--- test/Driver/XRay/xray-instrument-os.c
+++ test/Driver/XRay/xray-instrument-os.c
@@ -1,4 +1,4 @@
 // RUN: not %clang -o /dev/null -v -fxray-instrument -c %s
 // XFAIL: -linux-
-// REQUIRES-ANY: amd64, x86_64, x86_64h, arm
+// REQUIRES-ANY: amd64, x86_64, x86_64h, arm, aarch64, arm64
 typedef int a;
Index: test/Driver/XRay/xray-instrument-cpu.c
===
--- test/Driver/XRay/xray-instrument-cpu.c
+++ test/Driver/XRay/xray-instrument-cpu.c
@@ -1,4 +1,4 @@
 // RUN: not %clang -o /dev/null -v -fxray-instrument -c %s
-// XFAIL: amd64-, x86_64-, x86_64h-, arm
+// XFAIL: amd64-, x86_64-, x86_64h-, arm, aarch64, arm64
 // REQUIRES: linux
 typedef int a;
Index: lib/Driver/Tools.cpp
===
--- lib/Driver/Tools.cpp
+++ lib/Driver/Tools.cpp
@@ -4900,14 +4900,20 @@
   if (Args.hasFlag(options::OPT_fxray_instrument,
options::OPT_fnoxray_instrument, false)) {
 const char *const XRayInstrumentOption = "-fxray-instrument";
-if (Triple.getOS() == llvm::Triple::Linux &&
-(Triple.getArch() == llvm::Triple::arm ||
- Triple.getArch() == llvm::Triple::x86_64)) {
-  // Supported.
-} else {
+if (Triple.getOS() == llvm::Triple::Linux)
+  switch (Triple.getArch()) {
+  case llvm::Triple::x86_64:
+  case llvm::Triple::arm:
+  case llvm::Triple::aarch64:
+// Supported.
+break;
+  default:
+D.Diag(diag::err_drv_clang_unsupported)
+<< (std::string(XRayInstrumentOption) + " on " + Triple.str());
+  }
+else
   D.Diag(diag::err_drv_clang_unsupported)
-  << (std::string(XRayInstrumentOption) + " on " + Triple.str());
-}
+  << (std::string(XRayInstrumentOption) + " on non-Linux target OS");
 CmdArgs.push_back(XRayInstrumentOption);
 if (const Arg *A =
 Args.getLastArg(options::OPT_fxray_instruction_threshold_,


Index: test/Driver/XRay/xray-instrument-os.c
===
--- test/Driver/XRay/xray-instrument-os.c
+++ test/Driver/XRay/xray-instrument-os.c
@@ -1,4 +1,4 @@
 // RUN: not %clang -o /dev/null -v -fxray-instrument -c %s
 // XFAIL: -linux-
-// REQUIRES-ANY: amd64, x86_64, x86_64h, arm
+// REQUIRES-ANY: amd64, x86_64, x86_64h, arm, aarch64, arm64
 typedef int a;
Index: test/Driver/XRay/xray-instrument-cpu.c
===
--- test/Driver/XRay/xray-instrument-cpu.c
+++ test/Driver/XRay/xray-instrument-cpu.c
@@ -1,4 +1,4 @@
 // RUN: not %clang -o /dev/null -v -fxray-instrument -c %s
-// XFAIL: amd64-, x86_64-, x86_64h-, arm
+// XFAIL: amd64-, x86_64-, x86_64h-, arm, aarch64, arm64
 // REQUIRES: linux
 typedef int a;
Index: lib/Driver/Tools.cpp
===
--- lib/Driver/Tools.cpp
+++ lib/Driver/Tools.cpp
@@ -4900,14 +4900,20 @@
   if (Args.hasFlag(options::OPT_fxray_instrument,
options::OPT_fnoxray_instrument, false)) {
 const char *const XRayInstrumentOption = "-fxray-instrument";
-if (Triple.getOS() == llvm::Triple::Linux &&
-(Triple.getArch() == llvm::Triple::arm ||
- Triple.getArch() == llvm::Triple::x86_64)) {
-  // Supported.
-} else {
+if (Triple.getOS() == llvm::Triple::Linux)
+  switch (Triple.getArch()) {
+  case llvm::Triple::x86_64:
+  case llvm::Triple::arm:
+  case llvm::Triple::aarch64:
+// Supported.
+break;
+  default:
+D.Diag(diag::err_drv_clang_unsupported)
+<< (std::string(XRayInstrumentOption) + " on " + Triple.str());
+  }
+else
   D.Diag(diag::err_drv_clang_unsupported)
-  << (std::string(XRayInstrumentOption) + " on " + Triple.str());
-}
+  << (std::string(XRayInstrumentOption) + " on non-Linux target OS");
 CmdArgs.push_back(XRayInstrumentOption);
 if (const Arg *A =
 Args.getLastArg(options::OPT_fxray_instruction_threshold_,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D26415: [XRay] Support AArch64 in Clang

[Serge Rogatch via cfe-commits]

rSerge added inline comments.



Comment at: lib/Driver/Tools.cpp:4903-4906
+if (Triple.getOS() != llvm::Triple::Linux)
+  D.Diag(diag::err_drv_clang_unsupported)
+  << (std::string(XRayInstrumentOption) + " on non-Linux target OS.");
+switch (Triple.getArch()) {

rSerge wrote:
> dberris wrote:
> > Did you need to put an `else` before the switch? I don't know what happens 
> > when you're on Windows non-supported platform -- will this cause two 
> > diagnostics to be printed?
> I thought that the statement after `D.Diag(diag::err_drv_clang_unsupported)` 
> will not be executed. Let me check...
Indeed, it was printing 2 diagnostics. I've added `else`.


https://reviews.llvm.org/D26415



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