Skip to content

[PS5][Driver] Pass user search paths to linker before implict ones #119875

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

Conversation

playstation-edd
Copy link
Contributor

Responsibility for setting up implicit library search paths was recently transferred to the PS5 driver (#109796). Prior to this, SIE private patches in lld performed this function. During the transition, I failed to maintain the order in which implicit and user-supplied search paths were supplied/considered. This change ensures user-supplied search paths appear before any implicit ones on the link line.

SIE tracker: TOOLCHAIN-17490

Responsibility for setting up implicit library search paths was recently
transferred to the PS5 driver (llvm#109796). Prior to this, SIE private
patches in lld performed this function. During the transition, I failed
to maintain the order in which implicit and user-supplied search paths
were supplied/considered. This change ensures user-supplied search
paths appear before any implicit ones on the link line.

SIE tracker: TOOLCHAIN-17490
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' labels Dec 13, 2024
@llvmbot
Copy link
Member

llvmbot commented Dec 13, 2024

@llvm/pr-subscribers-clang

Author: Edd Dawson (playstation-edd)

Changes

Responsibility for setting up implicit library search paths was recently transferred to the PS5 driver (llvm#109796). Prior to this, SIE private patches in lld performed this function. During the transition, I failed to maintain the order in which implicit and user-supplied search paths were supplied/considered. This change ensures user-supplied search paths appear before any implicit ones on the link line.

SIE tracker: TOOLCHAIN-17490


Full diff: https://github.com/llvm/llvm-project/pull/119875.diff

2 Files Affected:

  • (modified) clang/lib/Driver/ToolChains/PS4CPU.cpp (+3-2)
  • (modified) clang/test/Driver/ps5-linker.c (+12-14)
diff --git a/clang/lib/Driver/ToolChains/PS4CPU.cpp b/clang/lib/Driver/ToolChains/PS4CPU.cpp
index c2eeb8f513066f..b59c89f107d816 100644
--- a/clang/lib/Driver/ToolChains/PS4CPU.cpp
+++ b/clang/lib/Driver/ToolChains/PS4CPU.cpp
@@ -361,9 +361,10 @@ void tools::PS5cpu::Linker::ConstructJob(Compilation &C, const JobAction &JA,
   if (StringRef Jobs = getLTOParallelism(Args, D); !Jobs.empty())
     AddLTOFlag(Twine("jobs=") + Jobs);
 
+  Args.AddAllArgs(CmdArgs, options::OPT_L);
   TC.AddFilePathLibArgs(Args, CmdArgs);
-  Args.addAllArgs(CmdArgs, {options::OPT_L, options::OPT_T_Group,
-                            options::OPT_s, options::OPT_t});
+  Args.addAllArgs(CmdArgs, {options::OPT_T_Group, options::OPT_s,
+                            options::OPT_t});
 
   if (Args.hasArg(options::OPT_Z_Xlinker__no_demangle))
     CmdArgs.push_back("--no-demangle");
diff --git a/clang/test/Driver/ps5-linker.c b/clang/test/Driver/ps5-linker.c
index 62aa3a40e455af..53f89a914f4fae 100644
--- a/clang/test/Driver/ps5-linker.c
+++ b/clang/test/Driver/ps5-linker.c
@@ -172,29 +172,27 @@
 // CHECK-SYSROOT: {{ld(\.exe)?}}"
 // CHECK-SYSROOT-SAME: "--sysroot=mysdk"
 
-// Test that "." is always added to library search paths. This is long-standing
-// behavior, unique to PlayStation toolchains.
-
-// RUN: %clang --target=x64_64-sie-ps5 %s -### 2>&1 | FileCheck --check-prefixes=CHECK-LDOT %s
-
-// CHECK-LDOT: {{ld(\.exe)?}}"
-// CHECK-LDOT-SAME: "-L."
-
-// Test that <sdk-root>/target/lib is added to library search paths, if it
-// exists and no --sysroot is specified. Also confirm that CRT objects are
-// found there.
+// Test implicit library search paths are supplied to the linker, after any
+// search paths specified by the user. <sdk-root>/target/lib is implicitly
+// added if it exists and no --sysroot is specified. CRT objects are found
+// there. "." is always implicitly added to library search paths. This is
+// long-standing behavior, unique to PlayStation toolchains.
 
 // RUN: rm -rf %t.dir && mkdir %t.dir
-// RUN: env SCE_PROSPERO_SDK_DIR=%t.dir %clang --target=x64_64-sie-ps5 %s -### 2>&1 | FileCheck --check-prefixes=CHECK-NO-TARGETLIB %s
-// RUN: env SCE_PROSPERO_SDK_DIR=%t.dir %clang --target=x64_64-sie-ps5 %s -### --sysroot=%t.dir 2>&1 | FileCheck --check-prefixes=CHECK-NO-TARGETLIB %s
+// RUN: env SCE_PROSPERO_SDK_DIR=%t.dir %clang --target=x64_64-sie-ps5 %s -### -Luser 2>&1 | FileCheck --check-prefixes=CHECK-NO-TARGETLIB %s
+// RUN: env SCE_PROSPERO_SDK_DIR=%t.dir %clang --target=x64_64-sie-ps5 %s -### -Luser --sysroot=%t.dir 2>&1 | FileCheck --check-prefixes=CHECK-NO-TARGETLIB %s
 
 // CHECK-NO-TARGETLIB: {{ld(\.exe)?}}"
+// CHECK-NO-TARGETLIB-SAME: "-Luser"
 // CHECK-NO-TARGETLIB-NOT: "-L{{.*[/\\]}}target/lib"
+// CHECK-NO-TARGETLIB-SAME: "-L."
 
 // RUN: mkdir -p %t.dir/target/lib
 // RUN: touch %t.dir/target/lib/crti.o
-// RUN: env SCE_PROSPERO_SDK_DIR=%t.dir %clang --target=x64_64-sie-ps5 %s -### 2>&1 | FileCheck --check-prefixes=CHECK-TARGETLIB %s
+// RUN: env SCE_PROSPERO_SDK_DIR=%t.dir %clang --target=x64_64-sie-ps5 %s -### -Luser 2>&1 | FileCheck --check-prefixes=CHECK-TARGETLIB %s
 
 // CHECK-TARGETLIB: {{ld(\.exe)?}}"
+// CHECK-TARGETLIB-SAME: "-Luser"
 // CHECK-TARGETLIB-SAME: "-L{{.*[/\\]}}target/lib"
+// CHECK-TARGETLIB-SAME: "-L."
 // CHECK-TARGETLIB-SAME: "{{.*[/\\]}}target{{/|\\\\}}lib{{/|\\\\}}crti.o"

@llvmbot
Copy link
Member

llvmbot commented Dec 13, 2024

@llvm/pr-subscribers-clang-driver

Author: Edd Dawson (playstation-edd)

Changes

Responsibility for setting up implicit library search paths was recently transferred to the PS5 driver (llvm#109796). Prior to this, SIE private patches in lld performed this function. During the transition, I failed to maintain the order in which implicit and user-supplied search paths were supplied/considered. This change ensures user-supplied search paths appear before any implicit ones on the link line.

SIE tracker: TOOLCHAIN-17490


Full diff: https://github.com/llvm/llvm-project/pull/119875.diff

2 Files Affected:

  • (modified) clang/lib/Driver/ToolChains/PS4CPU.cpp (+3-2)
  • (modified) clang/test/Driver/ps5-linker.c (+12-14)
diff --git a/clang/lib/Driver/ToolChains/PS4CPU.cpp b/clang/lib/Driver/ToolChains/PS4CPU.cpp
index c2eeb8f513066f..b59c89f107d816 100644
--- a/clang/lib/Driver/ToolChains/PS4CPU.cpp
+++ b/clang/lib/Driver/ToolChains/PS4CPU.cpp
@@ -361,9 +361,10 @@ void tools::PS5cpu::Linker::ConstructJob(Compilation &C, const JobAction &JA,
   if (StringRef Jobs = getLTOParallelism(Args, D); !Jobs.empty())
     AddLTOFlag(Twine("jobs=") + Jobs);
 
+  Args.AddAllArgs(CmdArgs, options::OPT_L);
   TC.AddFilePathLibArgs(Args, CmdArgs);
-  Args.addAllArgs(CmdArgs, {options::OPT_L, options::OPT_T_Group,
-                            options::OPT_s, options::OPT_t});
+  Args.addAllArgs(CmdArgs, {options::OPT_T_Group, options::OPT_s,
+                            options::OPT_t});
 
   if (Args.hasArg(options::OPT_Z_Xlinker__no_demangle))
     CmdArgs.push_back("--no-demangle");
diff --git a/clang/test/Driver/ps5-linker.c b/clang/test/Driver/ps5-linker.c
index 62aa3a40e455af..53f89a914f4fae 100644
--- a/clang/test/Driver/ps5-linker.c
+++ b/clang/test/Driver/ps5-linker.c
@@ -172,29 +172,27 @@
 // CHECK-SYSROOT: {{ld(\.exe)?}}"
 // CHECK-SYSROOT-SAME: "--sysroot=mysdk"
 
-// Test that "." is always added to library search paths. This is long-standing
-// behavior, unique to PlayStation toolchains.
-
-// RUN: %clang --target=x64_64-sie-ps5 %s -### 2>&1 | FileCheck --check-prefixes=CHECK-LDOT %s
-
-// CHECK-LDOT: {{ld(\.exe)?}}"
-// CHECK-LDOT-SAME: "-L."
-
-// Test that <sdk-root>/target/lib is added to library search paths, if it
-// exists and no --sysroot is specified. Also confirm that CRT objects are
-// found there.
+// Test implicit library search paths are supplied to the linker, after any
+// search paths specified by the user. <sdk-root>/target/lib is implicitly
+// added if it exists and no --sysroot is specified. CRT objects are found
+// there. "." is always implicitly added to library search paths. This is
+// long-standing behavior, unique to PlayStation toolchains.
 
 // RUN: rm -rf %t.dir && mkdir %t.dir
-// RUN: env SCE_PROSPERO_SDK_DIR=%t.dir %clang --target=x64_64-sie-ps5 %s -### 2>&1 | FileCheck --check-prefixes=CHECK-NO-TARGETLIB %s
-// RUN: env SCE_PROSPERO_SDK_DIR=%t.dir %clang --target=x64_64-sie-ps5 %s -### --sysroot=%t.dir 2>&1 | FileCheck --check-prefixes=CHECK-NO-TARGETLIB %s
+// RUN: env SCE_PROSPERO_SDK_DIR=%t.dir %clang --target=x64_64-sie-ps5 %s -### -Luser 2>&1 | FileCheck --check-prefixes=CHECK-NO-TARGETLIB %s
+// RUN: env SCE_PROSPERO_SDK_DIR=%t.dir %clang --target=x64_64-sie-ps5 %s -### -Luser --sysroot=%t.dir 2>&1 | FileCheck --check-prefixes=CHECK-NO-TARGETLIB %s
 
 // CHECK-NO-TARGETLIB: {{ld(\.exe)?}}"
+// CHECK-NO-TARGETLIB-SAME: "-Luser"
 // CHECK-NO-TARGETLIB-NOT: "-L{{.*[/\\]}}target/lib"
+// CHECK-NO-TARGETLIB-SAME: "-L."
 
 // RUN: mkdir -p %t.dir/target/lib
 // RUN: touch %t.dir/target/lib/crti.o
-// RUN: env SCE_PROSPERO_SDK_DIR=%t.dir %clang --target=x64_64-sie-ps5 %s -### 2>&1 | FileCheck --check-prefixes=CHECK-TARGETLIB %s
+// RUN: env SCE_PROSPERO_SDK_DIR=%t.dir %clang --target=x64_64-sie-ps5 %s -### -Luser 2>&1 | FileCheck --check-prefixes=CHECK-TARGETLIB %s
 
 // CHECK-TARGETLIB: {{ld(\.exe)?}}"
+// CHECK-TARGETLIB-SAME: "-Luser"
 // CHECK-TARGETLIB-SAME: "-L{{.*[/\\]}}target/lib"
+// CHECK-TARGETLIB-SAME: "-L."
 // CHECK-TARGETLIB-SAME: "{{.*[/\\]}}target{{/|\\\\}}lib{{/|\\\\}}crti.o"

Copy link

github-actions bot commented Dec 13, 2024

✅ With the latest revision this PR passed the C/C++ code formatter.

@wjristow wjristow self-requested a review December 19, 2024 00:43
Copy link
Collaborator

@wjristow wjristow left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@playstation-edd playstation-edd merged commit c616fdc into llvm:main Dec 19, 2024
8 checks passed
@playstation-edd playstation-edd deleted the ps5-user-lib-paths-before-implicit-lib-paths branch December 19, 2024 09:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants