Move versioned Apple LLVM targets from rustc_target to rustc_codegen_ssa
#131037
Conversation
The OS version depends on the deployment target environment variables, the access of which we want to move to later in the compilation pipeline that has access to more information, for example `env_depinfo`.
rustbot
added
O-apple
|
These commits modify compiler targets. Some changes occurred in compiler/rustc_codegen_cranelift cc @bjorn3 |
This comment has been minimized.
This comment has been minimized.
| @@ -259,7 +260,7 @@ impl CodegenBackend for CraneliftCodegenBackend { | |||
| } | |||
|
|
|||
| fn target_triple(sess: &Session) -> target_lexicon::Triple { | |||
| match sess.target.llvm_target.parse() { | |||
| match versioned_llvm_target(sess).parse() { | |||
There was a problem hiding this comment.
This should probably remain unchanged. Cranelift currently doesn't consume the deployment target and cranelift-object may well expect it to be passed in separately in the future. In general Cranelift barely takes any information from the target triple. Only the cpu architecture and default calling convention for the OS currently. Even the object file format dependent way to lower TLS is passed in using a separate flag.
There was a problem hiding this comment.
Yeah, we talked about this elsewhere, I did it like this for now so that the commit was completely free of functional changes.
In particular, I think passing an unversioned target triple to target-lexicon would result in an error here, as their current parsing code for macosx assumes that the version is always present.
There was a problem hiding this comment.
I'll try to whip up a PR to fix this in target-lexicon, but that shouldn't block this PR IMO.
madsmtm
force-pushed
the
move-llvm-target-versioning
branch
from
c4b18fc
to
d472728
Compare
Fully specified LLVM targets contain the OS version on macOS/iOS/tvOS/watchOS/visionOS, and this version depends on the deployment target environment variables like
MACOSX_DEPLOYMENT_TARGET,IPHONEOS_DEPLOYMENT_TARGETetc.We would like to move this to later in the compilation pipeline, both because it feels impure to access environment variables when fetching target information, but mostly because we need access to more information from #130883 to do #118204. See also #129342 (comment) for some discussion.
The first commit does the actual refactor, it should be a non-functional change, the second commit adds diagnostics for invalid deployment targets, which are now possible to do because we have access to the session.
Tested with the same commands as in #130435.
r? @petrochenkov