Skip to content

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.

@github-actions
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
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