[PATCH] D26415: [XRay] Support AArch64 in Clang
This revision was automatically updated to reflect the committed changes.
Closed by commit rL287518: [XRay] Support AArch64 in Clang (authored by
dberris).
Changed prior to commit:
https://reviews.llvm.org/D26415?vs=78216=78685#toc
Repository:
rL LLVM
https://reviews.llvm.org/D26415
Files:
cfe/trunk/lib/Driver/Tools.cpp
cfe/trunk/test/Driver/XRay/xray-instrument-cpu.c
cfe/trunk/test/Driver/XRay/xray-instrument-os.c
Index: cfe/trunk/test/Driver/XRay/xray-instrument-cpu.c
===
--- cfe/trunk/test/Driver/XRay/xray-instrument-cpu.c
+++ cfe/trunk/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: cfe/trunk/test/Driver/XRay/xray-instrument-os.c
===
--- cfe/trunk/test/Driver/XRay/xray-instrument-os.c
+++ cfe/trunk/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: cfe/trunk/lib/Driver/Tools.cpp
===
--- cfe/trunk/lib/Driver/Tools.cpp
+++ cfe/trunk/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: cfe/trunk/test/Driver/XRay/xray-instrument-cpu.c
===
--- cfe/trunk/test/Driver/XRay/xray-instrument-cpu.c
+++ cfe/trunk/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: cfe/trunk/test/Driver/XRay/xray-instrument-os.c
===
--- cfe/trunk/test/Driver/XRay/xray-instrument-os.c
+++ cfe/trunk/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: cfe/trunk/lib/Driver/Tools.cpp
===
--- cfe/trunk/lib/Driver/Tools.cpp
+++ cfe/trunk/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
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] D26415: [XRay] Support AArch64 in Clang
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
[PATCH] D26415: [XRay] Support AArch64 in Clang
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
dberris 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()) {
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?
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
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 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
dberris 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)) {
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 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.
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
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
> > 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
dberris 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)) {
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
> 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.
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
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 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
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
