Skip to content

[clang][AIX] Strip unknown environment component for per target runtime directory #140850

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 5 commits into from
May 24, 2025

Conversation

jakeegan
Copy link
Member

@jakeegan jakeegan commented May 21, 2025

Previously, when the triple is powerpc-ibm-aix-unknown, the driver fails to find subdirectory lib/powerpc-ibm-aix.

This ensures the correct runtime path is found if the triple has the -unknown environment component attached.

Copy link

github-actions bot commented May 21, 2025

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

@DanielCChen
Copy link
Contributor

A more general question:
It seems neither compile-rt nor flang-rt uses getTargetSubDirPath, so we didn't need to strip to the "base" triple.
Sanitizer uses it apparently. Is it by design? @daltenty

@daltenty
Copy link
Member

A more general question: It seems neither compile-rt nor flang-rt uses getTargetSubDirPath, so we didn't need to strip to the "base" triple. Sanitizer uses it apparently. Is it by design? @daltenty

I doubt it, this code is extremely messy and there's duplication everywhere, so I suspect that diverged unintentionally

@DanielCChen
Copy link
Contributor

A more general question: It seems neither compile-rt nor flang-rt uses getTargetSubDirPath, so we didn't need to strip to the "base" triple. Sanitizer uses it apparently. Is it by design? @daltenty

I doubt it, this code is extremely messy and there's duplication everywhere, so I suspect that diverged unintentionally

Yeah, I think I figured it out. I updated my inline comments to suggest a change.

It seems getTargetSubDirPath appends the triple dir to the argument. getRuntimePath calls getTargetSubDirPath with the argument of resource-dir/lib. Others pass different argument. For example, getStdlibIncludePath calls getTargetSubDirPath with ..../include.

@DanielCChen
Copy link
Contributor

A more general question:
It seems neither compile-rt nor flang-rt uses getTargetSubDirPath, so we didn't need to strip to the "base" triple.
Sanitizer uses it apparently. Is it by design?

The reason we didn't need to change getTargetSubDirPath is because we striped the triple in its caller, getRuntimePath.
I think the best way is to do it in getTargetSubDirPaht instead of getRuntimePath as I suggested in my inline comment.

@jakeegan jakeegan changed the title [clang][AIX] Handle triple environment component for per target runtime directory [clang][AIX] Strip unknown environment component for per target runtime directory May 23, 2025
@jakeegan jakeegan marked this pull request as ready for review May 23, 2025 02:21
@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 23, 2025
@llvmbot
Copy link
Member

llvmbot commented May 23, 2025

@llvm/pr-subscribers-clang-driver

Author: Jake Egan (jakeegan)

Changes

Previously, when the triple is powerpc-ibm-aix-unknown, the driver fails to find subdirectory lib/powerpc-ibm-aix.

This ensures the correct runtime path is found if the triple has the -unknown environment component attached.


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

2 Files Affected:

  • (modified) clang/lib/Driver/ToolChain.cpp (+9)
  • (modified) clang/test/Driver/aix-print-runtime-dir.c (+5)
diff --git a/clang/lib/Driver/ToolChain.cpp b/clang/lib/Driver/ToolChain.cpp
index 664aafad0f680..db37dccbd40b8 100644
--- a/clang/lib/Driver/ToolChain.cpp
+++ b/clang/lib/Driver/ToolChain.cpp
@@ -935,6 +935,15 @@ 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 (auto Path = getPathForTriple(AIXTriple))
+      return *Path;
+  }
+
   if (T.isOSzOS() &&
       (!T.getOSVersion().empty() || !T.getEnvironmentVersion().empty())) {
     // Build the triple without version information
diff --git a/clang/test/Driver/aix-print-runtime-dir.c b/clang/test/Driver/aix-print-runtime-dir.c
index ffa4d15c21208..16fe59c918804 100644
--- a/clang/test/Driver/aix-print-runtime-dir.c
+++ b/clang/test/Driver/aix-print-runtime-dir.c
@@ -16,6 +16,11 @@
 // RUN:        -resource-dir=%S/Inputs/resource_dir_with_per_target_subdir\
 // RUN:      | FileCheck --check-prefix=PRINT-RUNTIME-DIR64-PER-TARGET %s
 
+// RUN: %clang -print-runtime-dir --target=powerpc-ibm-aix-unknown \
+// RUN:        -resource-dir=%S/Inputs/resource_dir_with_per_target_subdir\
+// RUN:      | FileCheck --check-prefix=PRINT-RUNTIME-DIR-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-DIR-UNKNOWN-ENV: lib{{/|\\}}powerpc-ibm-aix

@llvmbot
Copy link
Member

llvmbot commented May 23, 2025

@llvm/pr-subscribers-clang

Author: Jake Egan (jakeegan)

Changes

Previously, when the triple is powerpc-ibm-aix-unknown, the driver fails to find subdirectory lib/powerpc-ibm-aix.

This ensures the correct runtime path is found if the triple has the -unknown environment component attached.


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

2 Files Affected:

  • (modified) clang/lib/Driver/ToolChain.cpp (+9)
  • (modified) clang/test/Driver/aix-print-runtime-dir.c (+5)
diff --git a/clang/lib/Driver/ToolChain.cpp b/clang/lib/Driver/ToolChain.cpp
index 664aafad0f680..db37dccbd40b8 100644
--- a/clang/lib/Driver/ToolChain.cpp
+++ b/clang/lib/Driver/ToolChain.cpp
@@ -935,6 +935,15 @@ 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 (auto Path = getPathForTriple(AIXTriple))
+      return *Path;
+  }
+
   if (T.isOSzOS() &&
       (!T.getOSVersion().empty() || !T.getEnvironmentVersion().empty())) {
     // Build the triple without version information
diff --git a/clang/test/Driver/aix-print-runtime-dir.c b/clang/test/Driver/aix-print-runtime-dir.c
index ffa4d15c21208..16fe59c918804 100644
--- a/clang/test/Driver/aix-print-runtime-dir.c
+++ b/clang/test/Driver/aix-print-runtime-dir.c
@@ -16,6 +16,11 @@
 // RUN:        -resource-dir=%S/Inputs/resource_dir_with_per_target_subdir\
 // RUN:      | FileCheck --check-prefix=PRINT-RUNTIME-DIR64-PER-TARGET %s
 
+// RUN: %clang -print-runtime-dir --target=powerpc-ibm-aix-unknown \
+// RUN:        -resource-dir=%S/Inputs/resource_dir_with_per_target_subdir\
+// RUN:      | FileCheck --check-prefix=PRINT-RUNTIME-DIR-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-DIR-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. Thanks.

@jakeegan jakeegan merged commit 6d1d937 into llvm:main May 24, 2025
11 checks passed
@jakeegan jakeegan deleted the dir_fix branch May 24, 2025 07:05
@llvm-ci
Copy link
Collaborator

llvm-ci commented May 24, 2025

LLVM Buildbot has detected a new failure on builder amdgpu-offload-rhel-9-cmake-build-only running on rocm-docker-rhel-9 while building clang at step 2 "update-annotated-scripts".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/205/builds/10303

Here is the relevant piece of the build log for the reference
Step 2 (update-annotated-scripts) failure: update (failure)
git version 2.43.5
fatal: unable to access 'https://github.com/llvm/llvm-zorg.git/': The requested URL returned error: 403
fatal: unable to access 'https://github.com/llvm/llvm-zorg.git/': The requested URL returned error: 403

sivan-shani pushed a commit to sivan-shani/llvm-project that referenced this pull request Jun 3, 2025
…me directory (llvm#140850)

Previously, when the triple is `powerpc-ibm-aix-unknown`, the driver
fails to find subdirectory `lib/powerpc-ibm-aix`.

This ensures the correct runtime path is found if the triple has the
-unknown environment component attached.
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.

5 participants