Skip to content
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

What minimum version of Apple Clang should we report supporting? #26851

Open
bradcray opened this issue Mar 6, 2025 · 5 comments
Open

What minimum version of Apple Clang should we report supporting? #26851

bradcray opened this issue Mar 6, 2025 · 5 comments

Comments

@bradcray
Copy link
Member

bradcray commented Mar 6, 2025

The GASNet team has reported that:

Based on this, it seems as though we should update our minimum version of Apple Clang in prereqs.rst (currently said to be 10 due to that being LLVM's documented minimum). But to what value?

@bradcray
Copy link
Member Author

bradcray commented Mar 6, 2025

Paul reports that Apple clang 12.0.0.12000032 is sufficient to build Chapel as of 66b41e8 and that it corresponds to Mac OS X 10.15 / Catalina, which was EOL'd in 2022. He also confirms that by updating to #26853, he can build with Apple Clang 11. So I would propose that we update our prereqs file to list 11 as the minimum rather than 10.

@PHHargrove
Copy link
Contributor

Expanding on what I reported about an hour ago in the llvm-project issue, there is a 1-line change which allows me to build Chapel's LLVM bits with Apple Clang 10 when CHPL_LLVM=NONE. Below is my change as a patch to the Chapel repo (as opposed to the llvm-project repo).
IMO, the most likely reason nobody in the LLVM project thought to address this deduction failure with an explicit template parameter is that nobody cares about Apple Clang 10. I only care enough at the moment because the obvious fix was bothering me enough I had to try it.

HOWEVER, that was not sufficient:

  1. Once past the LLVM portions, the qthreads build failed on C11 atomics. It looks like a compiler/header bug one might work-around with a cast to discard a const qualifier.
  2. Additionally, an attempt to build with CHPL_LLVM=bundled failed elsewhere with another std::optional which likely just needs the type made explicit. However, I am not heading down that rabbit hole, having satisfied my intellectual curiosity with the patch below.

So, unless somebody wants to address the remaining issues I've identified, I think it makes sense to document Apple Clang 11 as the minimum, as Brad has suggested.

diff --git a/third-party/llvm/llvm-src/lib/Support/Program.cpp b/third-party/llvm/llvm-src/lib/Support/Program.cpp
index 181f68c..47f24f8 100644
--- a/third-party/llvm/llvm-src/lib/Support/Program.cpp
+++ b/third-party/llvm/llvm-src/lib/Support/Program.cpp
@@ -43,7 +43,7 @@ int sys::ExecuteAndWait(StringRef Program, ArrayRef<StringRef> Args,
     if (ExecutionFailed)
       *ExecutionFailed = false;
     ProcessInfo Result = Wait(
-        PI, SecondsToWait == 0 ? std::nullopt : std::optional(SecondsToWait),
+        PI, SecondsToWait == 0 ? std::nullopt : std::optional<unsigned>(SecondsToWait),
         ErrMsg, ProcStat);
     return Result.ReturnCode;
   }

@mppf
Copy link
Member

mppf commented Mar 6, 2025

I did a little poking around to see if we required clang 11 what trouble that might cause.

Amazon Linux 2 (EOL June 2025), Debian Bullseye (EOL June 2026) use clang 11. Also, when working with the LLVM backend, we support LLVM 11 and newer, and in this setting, we need the corresponding version of clang to be built.

LLVM 11 and clang 11 were released in 2020. I can't find any official information from the LLVM project about how many releases they support. AFAIK they basically only support the current release (in that, they don't seem to make point releases for older releases).

@mppf
Copy link
Member

mppf commented Mar 6, 2025

Given all of that, and the issue with Apple clang 10, my recommendation is to document that we require clang 11 when using clang, including Apple clang. That makes the requirement simpler (since it also covers our minimum version for the LLVM backend) even though it's not strictly necessary.

@bradcray
Copy link
Member Author

bradcray commented Mar 6, 2025

That sounds good to me, thanks for the additional investigation, Michael! Now we just need someone to own this glamorous task. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants