[Driver][xray] Allow XRay on Apple Silicon

[ilammy]

Summary

Codegen can handle XRay for AArch64, tell the driver to allow it.

[MaskRay]

clang/test/Driver/XRay/xray-instrument-macos.c

1

--target=

Avoid -target (legacy) for new tests.

[ilammy]

Addressing feedback by @MaskRay:

• Moved // REQUIRES: directive to the top of the test file
• Replaced legacy -target option with proper --target in tests
• Also rebased on updated trunk

[MaskRay]

This is a kinda odd case. Normally we should enable the option in the driver when the feature actually works.
However, Triple.isMacOSX() is allowed before the feature actually works and compiler-rt/test/xray/lit.cfg.py tests it.

clang/test/Driver/XRay/ is broken and does not follow the practice testing driver changes.
clang/test/Driver should not test code generation but XRay/ tests do.

The targets aren't really enabled since REQUIRES: aarch64 || x86_64 || x86_64h lines specified lit features aren't available.

clang/lib/Driver/XRayArgs.cpp

66

The file was written in a non-conventional way. I just cleaned up the file a bit.

[ilammy]

In D145849#4412286, @MaskRay wrote:

However, Triple.isMacOSX() is allowed before the feature actually works and compiler-rt/test/xray/lit.cfg.py tests it.

I have some patches for compiler-rt cooking, fixing the build for AArch64. Should those go in first, before clang is ultimately allowed to use -fxray-instrument when targeting AArch64, so that everything works out of the box?

For me, Clang is able to compile binaries with XRay sections (with this change), but I have compiler-rt disabled and not linked in, since it's not possible to build it for AArch64 yet.

I thought it could be done separately. I was not aware of those tests in compiler-rt, checking all archs.

I guess the build should be fixed first, then this change should simultaneously allow clang to target AArch64 and update compiler-rt's expectations of what works.

How does that sound to you?

[MaskRay]

In D145849#4430870, @ilammy wrote:

In D145849#4412286, @MaskRay wrote:

However, Triple.isMacOSX() is allowed before the feature actually works and compiler-rt/test/xray/lit.cfg.py tests it.

I have some patches for compiler-rt cooking, fixing the build for AArch64. Should those go in first, before clang is ultimately allowed to use -fxray-instrument when targeting AArch64, so that everything works out of the box?

For me, Clang is able to compile binaries with XRay sections (with this change), but I have compiler-rt disabled and not linked in, since it's not possible to build it for AArch64 yet.

I thought it could be done separately. I was not aware of those tests in compiler-rt, checking all archs.

I guess the build should be fixed first, then this change should simultaneously allow clang to target AArch64 and update compiler-rt's expectations of what works.

How does that sound to you?

Since Mach-O porting is supposed to be done soon, I think landing this change is fine. This patch should make fixing the underlying issue easier as we don't need to locally patch XRayArgs.cpp...

I think D152661 is very much desired and ideally landed before other fixes.

[MaskRay]

In D145849#4436623, @MaskRay wrote:

In D145849#4430870, @ilammy wrote:

In D145849#4412286, @MaskRay wrote:

However, Triple.isMacOSX() is allowed before the feature actually works and compiler-rt/test/xray/lit.cfg.py tests it.

I have some patches for compiler-rt cooking, fixing the build for AArch64. Should those go in first, before clang is ultimately allowed to use -fxray-instrument when targeting AArch64, so that everything works out of the box?

For me, Clang is able to compile binaries with XRay sections (with this change), but I have compiler-rt disabled and not linked in, since it's not possible to build it for AArch64 yet.

I thought it could be done separately. I was not aware of those tests in compiler-rt, checking all archs.

I guess the build should be fixed first, then this change should simultaneously allow clang to target AArch64 and update compiler-rt's expectations of what works.

How does that sound to you?

Since Mach-O porting is supposed to be done soon, I think landing this change is fine. This patch should make fixing the underlying issue easier as we don't need to locally patch XRayArgs.cpp...

I think D152661 is very much desired and ideally landed before other fixes.

Needs to be rebased first. Hold on. I'll improve the xray-instrument* tests soon and you may want to git pull:)

clang/lib/Driver/XRayArgs.cpp

if (Triple.getOS() == llvm::Triple::Linux) {
     case llvm::Triple::mips64:
     case llvm::Triple::mips64el:
       break;
     default:
       D.Diag(diag::err_drv_clang_unsupported)
           << (std::string(XRayInstrumentOption) + " on " + Triple.str());
     }
   } else if (Triple.isOSFreeBSD() || Triple.isOSOpenBSD() ||
-             Triple.isOSNetBSD() || Triple.isMacOSX()) {
+             Triple.isOSNetBSD()) {
     if (Triple.getArch() != llvm::Triple::x86_64) {
       D.Diag(diag::err_drv_clang_unsupported)
           << (std::string(XRayInstrumentOption) + " on " + Triple.str());
     }
+  } else if (Triple.isMacOSX()) {
+    switch (Triple.getArch()) {
+    case llvm::Triple::x86_64:
+    case llvm::Triple::aarch64:
+      break;
+    default:
+      D.Diag(diag::err_drv_clang_unsupported)
+          << (std::string(XRayInstrumentOption) + " on " + Triple.str());
+    }
   } else if (Triple.getOS() == llvm::Triple::Fuchsia) {
     switch (Triple.getArch()) {
     case llvm::Triple::x86_64:
     case llvm::Triple::aarch64:
       break;
     default:
       D.Diag(diag::err_drv_clang_unsupported)
           << (std::string(XRayInstrumentOption) + " on " + Triple.str());

clang/test/Driver/XRay/xray-instrument-macos.c

+// RUN: %clang -o /dev/null -v -fxray-instrument -target aarch64-apple-darwin20 -c %s
 // RUN: %clang -o /dev/null -v -fxray-instrument -target x86_64-apple-macos10.11 -c %s
 // RUN: %clang -o /dev/null -v -fxray-instrument -target x86_64-apple-darwin15 -c %s
-// REQUIRES: x86_64 || x86_64h
+// REQUIRES: aarch64 || x86_64 || x86_64h
 typedef int a;

clang/test/Driver/XRay/xray-instrument-os.c

 // RUN: not %clang -o /dev/null -v -fxray-instrument -c %s
-// XFAIL: target={{.*-(linux|freebsd).*}}, target=x86_64-apple-{{(darwin|macos).*}}
+// XFAIL: target={{.*-(linux|freebsd).*}}, target={{(aarch64|x86_64)-apple-(darwin|macos).*}}
 // REQUIRES: target={{(amd64|x86_64|x86_64h|arm|aarch64|arm64)-.*}}
 typedef int a;