Skip to content

[clang][AIX] Fix -print-runtime-dir fallback on AIX #141439

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

Merged
merged 4 commits into from
Jun 2, 2025

Conversation

jakeegan
Copy link
Member

If the runtime path is not found (by getTargetSubDirPath()), since per target runtime directory is enabled on AIX, we should fall back to the target subdirectory rather than the OS subdirectory.

Copy link

github-actions bot commented May 26, 2025

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

@jakeegan jakeegan marked this pull request as ready for review May 26, 2025 01:05
@jakeegan jakeegan requested review from DanielCChen May 26, 2025 01:05
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' labels May 26, 2025
@jakeegan jakeegan requested a review from daltenty May 26, 2025 01:06
@llvmbot
Copy link
Member

llvmbot commented May 26, 2025

@llvm/pr-subscribers-clang

@llvm/pr-subscribers-clang-driver

Author: Jake Egan (jakeegan)

Changes

If the runtime path is not found (by getTargetSubDirPath()), since per target runtime directory is enabled on AIX, we should fall back to the target subdirectory rather than the OS subdirectory.


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

2 Files Affected:

  • (modified) clang/lib/Driver/ToolChain.cpp (+10-13)
  • (modified) clang/test/Driver/aix-print-runtime-dir.c (-9)
diff --git a/clang/lib/Driver/ToolChain.cpp b/clang/lib/Driver/ToolChain.cpp
index ce302b308fd19..024d2534dd248 100644
--- a/clang/lib/Driver/ToolChain.cpp
+++ b/clang/lib/Driver/ToolChain.cpp
@@ -933,11 +933,16 @@ ToolChain::getTargetSubDirPath(StringRef BaseDir) const {
   if (auto Path = getPathForTriple(T))
     return *Path;
 
-  if (T.isOSAIX() && T.getEnvironment() == Triple::UnknownEnvironment) {
-    // Strip unknown environment from the triple.
-    const llvm::Triple AIXTriple(
-        llvm::Triple(T.getArchName(), T.getVendorName(),
-                     llvm::Triple::getOSTypeName(T.getOS())));
+  if (T.isOSAIX()) {
+    llvm::Triple AIXTriple;
+    if (T.getEnvironment() == Triple::UnknownEnvironment) {
+      // Strip unknown environment from the triple.
+      AIXTriple = llvm::Triple(T.getArchName(), T.getVendorName(),
+                               llvm::Triple::getOSTypeName(T.getOS()));
+    } else {
+      // Get the triple without the OS version.
+      AIXTriple = getTripleWithoutOSVersion();
+    }
     if (auto Path = getPathForTriple(AIXTriple))
       return *Path;
   }
@@ -987,14 +992,6 @@ std::optional<std::string> ToolChain::getRuntimePath() const {
   if (Triple.isOSDarwin())
     return {};
 
-  // For AIX, get the triple without the OS version.
-  if (Triple.isOSAIX()) {
-    const llvm::Triple &TripleWithoutVersion = getTripleWithoutOSVersion();
-    llvm::sys::path::append(P, TripleWithoutVersion.str());
-    if (getVFS().exists(P))
-      return std::string(P);
-    return {};
-  }
   llvm::sys::path::append(P, Triple.str());
   return std::string(P);
 }
diff --git a/clang/test/Driver/aix-print-runtime-dir.c b/clang/test/Driver/aix-print-runtime-dir.c
index ef133728d731f..4740129442540 100644
--- a/clang/test/Driver/aix-print-runtime-dir.c
+++ b/clang/test/Driver/aix-print-runtime-dir.c
@@ -1,13 +1,5 @@
 // Test output of -print-runtime-dir on AIX
 
-// RUN: %clang -print-runtime-dir --target=powerpc-ibm-aix \
-// RUN:        -resource-dir=%S/Inputs/resource_dir \
-// RUN:      | FileCheck --check-prefix=PRINT-RUNTIME-DIR %s
-
-// RUN: %clang -print-runtime-dir --target=powerpc64-ibm-aix \
-// RUN:        -resource-dir=%S/Inputs/resource_dir \
-// RUN:      | FileCheck --check-prefix=PRINT-RUNTIME-DIR %s
-
 // RUN: %clang -print-runtime-dir --target=powerpc-ibm-aix \
 // RUN:        -resource-dir=%S/Inputs/resource_dir_with_per_target_subdir\
 // RUN:      | FileCheck --check-prefix=PRINT-RUNTIME-DIR32-PER-TARGET %s
@@ -24,7 +16,6 @@
 // RUN:        -resource-dir=%S/Inputs/resource_dir_with_per_target_subdir \
 // RUN:      | FileCheck --check-prefix=PRINT-RUNTIME-DIR64-UNKNOWN-ENV %s 
 
-// PRINT-RUNTIME-DIR: lib{{/|\\}}aix{{$}}
 // PRINT-RUNTIME-DIR32-PER-TARGET: lib{{/|\\}}powerpc-ibm-aix{{$}}
 // PRINT-RUNTIME-DIR64-PER-TARGET: lib{{/|\\}}powerpc64-ibm-aix{{$}}
 // PRINT-RUNTIME-DIR32-UNKNOWN-ENV: lib{{/|\\}}powerpc-ibm-aix

Copy link
Contributor

@DanielCChen DanielCChen left a comment

Choose a reason for hiding this comment

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

LGTM with a minor nit comment.

@DanielCChen
Copy link
Contributor

Another related question but not necessarily in the scope of this PR:
It seems both aix-ld.c and aix-rtlib.c still have the "old" OS path in test. They all pass though after we change the default to per target for libclang_rt*.a. Is it expected? If so, should the change of test in this PR void? @daltenty

Copy link
Contributor

@DanielCChen DanielCChen left a comment

Choose a reason for hiding this comment

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

LGTM.
Thanks.

@jakeegan jakeegan merged commit 0a68a9d into llvm:main Jun 2, 2025
11 checks passed
@jakeegan jakeegan deleted the rt-dir-fallback branch June 2, 2025 17:26
sallto pushed a commit to sallto/llvm-project that referenced this pull request Jun 3, 2025
If the runtime path is not found (by getTargetSubDirPath()), since per
target runtime directory is enabled on AIX, we should fall back to the
target subdirectory rather than the OS subdirectory.
DhruvSrivastavaX pushed a commit to DhruvSrivastavaX/lldb-for-aix that referenced this pull request Jun 12, 2025
If the runtime path is not found (by getTargetSubDirPath()), since per
target runtime directory is enabled on AIX, we should fall back to the
target subdirectory rather than the OS subdirectory.
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