-
Notifications
You must be signed in to change notification settings - Fork 12.6k
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
[PS4/PS5][Driver] Allow -static in PlayStation drivers #102020
[PS4/PS5][Driver] Allow -static in PlayStation drivers #102020
Conversation
On PlayStation, allow users to supply -static to the linker, via the driver. An initial step. Later changes will have the PS5 driver supply additional options to the linker, if and when -static is passed. SIE tracker: TOOLCHAIN-16704
@llvm/pr-subscribers-clang-driver @llvm/pr-subscribers-clang Author: Edd Dawson (playstation-edd) ChangesOn PlayStation, allow users to supply -static to the linker, via the driver. An initial step. Later changes will have the PS5 driver supply additional options to the linker, if and when -static is passed. SIE tracker: TOOLCHAIN-16704 Full diff: https://github.com/llvm/llvm-project/pull/102020.diff 5 Files Affected:
diff --git a/clang/lib/Driver/ToolChains/PS4CPU.cpp b/clang/lib/Driver/ToolChains/PS4CPU.cpp
index a9e612c44da06..3da9a280fd129 100644
--- a/clang/lib/Driver/ToolChains/PS4CPU.cpp
+++ b/clang/lib/Driver/ToolChains/PS4CPU.cpp
@@ -141,6 +141,9 @@ void tools::PS4cpu::Linker::ConstructJob(Compilation &C, const JobAction &JA,
if (Args.hasArg(options::OPT_pie))
CmdArgs.push_back("-pie");
+
+ if (Args.hasArg(options::OPT_static))
+ CmdArgs.push_back("--static");
if (Args.hasArg(options::OPT_rdynamic))
CmdArgs.push_back("-export-dynamic");
if (Args.hasArg(options::OPT_shared))
@@ -238,6 +241,8 @@ void tools::PS5cpu::Linker::ConstructJob(Compilation &C, const JobAction &JA,
if (Args.hasArg(options::OPT_pie))
CmdArgs.push_back("-pie");
+ if (Args.hasArg(options::OPT_static))
+ CmdArgs.push_back("--static");
if (Args.hasArg(options::OPT_rdynamic))
CmdArgs.push_back("-export-dynamic");
if (Args.hasArg(options::OPT_shared))
@@ -316,10 +321,6 @@ toolchains::PS4PS5Base::PS4PS5Base(const Driver &D, const llvm::Triple &Triple,
const ArgList &Args, StringRef Platform,
const char *EnvVar)
: Generic_ELF(D, Triple, Args) {
- if (Args.hasArg(clang::driver::options::OPT_static))
- D.Diag(clang::diag::err_drv_unsupported_opt_for_target)
- << "-static" << Platform;
-
// Determine where to find the PS4/PS5 libraries.
// If -isysroot was passed, use that as the SDK base path.
// If not, we use the EnvVar if it exists; otherwise use the driver's
diff --git a/clang/test/Driver/ps4-linker.c b/clang/test/Driver/ps4-linker.c
index 2a095d660bf36..47a515158fe81 100644
--- a/clang/test/Driver/ps4-linker.c
+++ b/clang/test/Driver/ps4-linker.c
@@ -1,3 +1,10 @@
+// Test that -static is forwarded to the linker
+
+// RUN: %clang --target=x86_64-scei-ps4 -static %s -### 2>&1 | FileCheck --check-prefixes=CHECK-STATIC %s
+
+// CHECK-STATIC: {{ld(\.exe)?}}"
+// CHECK-STATIC-SAME: "--static"
+
// Test the driver's control over the JustMyCode behavior with linker flags.
// RUN: %clang --target=x86_64-scei-ps4 -fjmc %s -### 2>&1 | FileCheck --check-prefixes=CHECK-LTO,CHECK-LIB %s
diff --git a/clang/test/Driver/ps4-pic.c b/clang/test/Driver/ps4-pic.c
index 0551a79cc1258..3072f379d13c6 100644
--- a/clang/test/Driver/ps4-pic.c
+++ b/clang/test/Driver/ps4-pic.c
@@ -23,8 +23,6 @@
// CHECK-DIAG-PIE: option '-fno-PIE' was ignored by the PS4 toolchain, using '-fPIC'
// CHECK-DIAG-pic: option '-fno-pic' was ignored by the PS4 toolchain, using '-fPIC'
// CHECK-DIAG-pie: option '-fno-pie' was ignored by the PS4 toolchain, using '-fPIC'
-//
-// CHECK-STATIC-ERR: unsupported option '-static' for target 'PS4'
// RUN: %clang -c %s -target x86_64-scei-ps4 -### 2>&1 \
// RUN: | FileCheck %s --check-prefix=CHECK-PIC2
@@ -79,9 +77,10 @@
// RUN: not %clang -c %s --target=x86_64-scei-ps4 -mdynamic-no-pic -fPIC -### 2>&1 \
// RUN: | FileCheck %s --check-prefix=CHECK-DYNAMIC-NO-PIC2
//
-// -static not supported at all.
-// RUN: not %clang -c %s --target=x86_64-scei-ps4 -static -### 2>&1 \
-// RUN: | FileCheck %s --check-prefix=CHECK-STATIC-ERR
+// The -static argument *doesn't* override PIC: -static only affects
+// linking, and -fPIC only affects code generation.
+// RUN: %clang -c %s -target x86_64-scei-ps4 -static -fPIC -### 2>&1 \
+// RUN: | FileCheck %s --check-prefix=CHECK-PIC2
//
// -fno-PIC etc. is obeyed if -mcmodel=kernel is also present.
// RUN: %clang -c %s -target x86_64-scei-ps4 -mcmodel=kernel -fno-PIC -### 2>&1 \
diff --git a/clang/test/Driver/ps5-linker.c b/clang/test/Driver/ps5-linker.c
index cf39d5bae97ac..01c0e20f970ed 100644
--- a/clang/test/Driver/ps5-linker.c
+++ b/clang/test/Driver/ps5-linker.c
@@ -1,3 +1,10 @@
+// Test that -static is forwarded to the linker
+
+// RUN: %clang --target=x86_64-scei-ps5 -static %s -### 2>&1 | FileCheck --check-prefixes=CHECK-STATIC %s
+
+// CHECK-STATIC: {{ld(\.exe)?}}"
+// CHECK-STATIC-SAME: "--static"
+
// Test the driver's control over the JustMyCode behavior with linker flags.
// RUN: %clang --target=x86_64-scei-ps5 -fjmc %s -### 2>&1 | FileCheck --check-prefixes=CHECK,CHECK-LIB %s
diff --git a/clang/test/Driver/ps5-pic.c b/clang/test/Driver/ps5-pic.c
index dcb1d50306ae1..2855bb5ec282a 100644
--- a/clang/test/Driver/ps5-pic.c
+++ b/clang/test/Driver/ps5-pic.c
@@ -23,8 +23,6 @@
// CHECK-DIAG-PIE: option '-fno-PIE' was ignored by the PS5 toolchain, using '-fPIC'
// CHECK-DIAG-pic: option '-fno-pic' was ignored by the PS5 toolchain, using '-fPIC'
// CHECK-DIAG-pie: option '-fno-pie' was ignored by the PS5 toolchain, using '-fPIC'
-//
-// CHECK-STATIC-ERR: unsupported option '-static' for target 'PS5'
// RUN: %clang -c %s -target x86_64-sie-ps5 -### 2>&1 \
// RUN: | FileCheck %s --check-prefix=CHECK-PIC2
@@ -79,9 +77,10 @@
// RUN: not %clang -c %s --target=x86_64-sie-ps5 -mdynamic-no-pic -fPIC -### 2>&1 \
// RUN: | FileCheck %s --check-prefix=CHECK-DYNAMIC-NO-PIC2
//
-// -static not supported at all.
-// RUN: not %clang -c %s --target=x86_64-sie-ps5 -static -### 2>&1 \
-// RUN: | FileCheck %s --check-prefix=CHECK-STATIC-ERR
+// The -static argument *doesn't* override PIC: -static only affects
+// linking, and -fPIC only affects code generation.
+// RUN: %clang -c %s -target x86_64-sie-ps5 -static -fPIC -### 2>&1 \
+// RUN: | FileCheck %s --check-prefix=CHECK-PIC2
//
// -fno-PIC etc. is obeyed if -mcmodel=kernel is also present.
// RUN: %clang -c %s -target x86_64-sie-ps5 -mcmodel=kernel -fno-PIC -### 2>&1 \
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
@@ -141,6 +141,9 @@ void tools::PS4cpu::Linker::ConstructJob(Compilation &C, const JobAction &JA, | |||
if (Args.hasArg(options::OPT_pie)) | |||
CmdArgs.push_back("-pie"); | |||
|
|||
|
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.
We don't use double blank lines as a logical separator.
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.
Oops - thanks. Removed.
@@ -141,6 +141,9 @@ void tools::PS4cpu::Linker::ConstructJob(Compilation &C, const JobAction &JA, | |||
if (Args.hasArg(options::OPT_pie)) | |||
CmdArgs.push_back("-pie"); | |||
|
|||
|
|||
if (Args.hasArg(options::OPT_static)) | |||
CmdArgs.push_back("--static"); |
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.
The linker option is almost always spelled as -static
.
Gnu.cpp:437 (ae623d1) demonstrates how these options work on Linux
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.
The linker option is almost always spelled as
-static
.
I have changed this.
FWIW, I took my cue from the --help
of ld.lld, assuming it would be a suitable authority.
Gnu.cpp:437 (ae623d1) demonstrates how these options work on Linux
Over coming changes we'll evolve to something more like that, but not exactly. The users of -static
on PlayStation are a small, distinct group, internal to SIE. Over the last decade or so -static
has - for better or worse - evolved into something very particular, here. Once all the functionality that should be in the PS driver is actually here, there's perhaps a chance that it can be rationalized.
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.
In lieu of Paul for a bit of time, LGTM.
I feel like the check for matching the linker-line could be made even stronger (one wonders how many options end with 'ld"', but it's probably good enough.
On PlayStation, allow users to supply -static to the linker, via the driver. An initial step. Later changes will have the PS5 driver supply additional options to the linker, if and when -static is passed. SIE tracker: TOOLCHAIN-16704
On PlayStation, allow users to supply -static to the linker, via the driver.
An initial step. Later changes will have the PS5 driver supply additional options to the linker, if and when -static is passed.
SIE tracker: TOOLCHAIN-16704