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

[jakeegan]

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]

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

[llvmbot]

@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

[DanielCChen]

LGTM with a minor nit comment.

[DanielCChen]

Nit: this effectively strips the OSVersion as well. Since we moved them together, may be explicitly say it in the comment to make it clear.

[jakeegan]

Moved the OS version comment to apply to the whole if statement

[DanielCChen]

Sorry Jake. This is not what I meant. I should have made it clearer.
I meant to change the original comment on line 937 to be Strip unknown environment and the OSVersion from the triple. and keep the comment at line 940 as is.

[DanielCChen]

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