-
Notifications
You must be signed in to change notification settings - Fork 13.3k
Enable xray support for Mac #140790
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Enable xray support for Mac #140790
Conversation
These commits modify compiler targets. |
This comment has been minimized.
This comment has been minimized.
//@ compile-flags: -Z instrument-xray --target x86_64-apple-darwin | ||
//@ compile-flags: -Z instrument-xray --target x86_64-pc-windows-msvc |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Question: do we have any smoke test for xray on x86_64-apple-darwin
and aarch64-apple-darwin
? As in:
- A smoke test that have some ability to observe xray being enabled or not, for
aarch64-apple-darwin
andx86_64-apple-darwin
targets. - A test check that the flag
-Z instrument-xray
is still unstable, but is no longer rejected onx86_64-apple-darwin
andaarch64-apple-darwin
respectively.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think both of these tests don't exist at the moment, I can add tests for this but there are two problems
- It seems that only llir is tested in
tests/codegen
and no asm, which cannot be used to check whether xray is enabled, because xray affects llvm output rather than input. i need some guidance how to write tests for asm output. - Add a test specifically for
instrument-xray
+x86_64-apple-darwin
/aarch64-apple-darwin
combination seems a bit strange to me, since this is not a feature we implement, but just an upstream feature. do we need to add such a test for each supported target?
Since this affects the Tier 1 @rustbot ping apple |
Hey Apple notification group! This issue or PR could use some Apple-specific (In case it's useful, here are some instructions for tackling these sorts of cc @BlackHoleFox @hkratz @inflation @madsmtm @nvzqz @shepmaster @thomcc |
@rustbot author (test nits) |
Reminder, once the PR becomes ready for a review, use |
#102921
Upstream has supported Mac for a while, let's enable it.
I've tested it on M4 and it generates nop sled correctly.