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

Fix compiling Pony programs on X86 MacOS when XCode 15 is the linker #4466

Merged
merged 2 commits into from
Oct 29, 2023

Conversation

SeanTAllen
Copy link
Member

No description provided.

@ponylang-main ponylang-main added the discuss during sync Should be discussed during an upcoming sync label Oct 11, 2023
@SeanTAllen SeanTAllen force-pushed the ldclassic branch 3 times, most recently from 73805cb to 95164d1 Compare October 11, 2023 14:50
@SeanTAllen SeanTAllen changed the title -ld_classic test Fix compiling Pony programs on MacOS when XCode 15 is the linker Oct 19, 2023
@SeanTAllen
Copy link
Member Author

@jemc is looking into testing this with XCode 15.

@jemc
Copy link
Member

jemc commented Oct 24, 2023

I haven't been able to reproduce the Sonoma / XCode 15 problem in ticket #4454 - the ld step is working fine for me on the M1 Mac mini without the change from this branch.

However, I also tested with the change from this branch, and it produces a lot of warnings during linking of the Pony program:

ld: warning: object file (/Users/main/Documents/code/ponyc/ring.o) was built for newer 'macOS' version (14.0) than being linked (13.0)
ld: warning: object file (/Users/main/Documents/code/ponyc/build/debug/libponyrt.a[2](actor.c.o)) was built for newer 'macOS' version (14.0) than being linked (13.0)
ld: warning: object file (/Users/main/Documents/code/ponyc/build/debug/libponyrt.a[3](messageq.c.o)) was built for newer 'macOS' version (14.0) than being linked (13.0)
ld: warning: object file (/Users/main/Documents/code/ponyc/build/debug/libponyrt.a[4](asio.c.o)) was built for newer 'macOS' version (14.0) than being linked (13.0)
ld: warning: object file (/Users/main/Documents/code/ponyc/build/debug/libponyrt.a[7](event.c.o)) was built for newer 'macOS' version (14.0) than being linked (13.0)
ld: warning: object file (/Users/main/Documents/code/ponyc/build/debug/libponyrt.a[9](kqueue.c.o)) was built for newer 'macOS' version (14.0) than being linked (13.0)
ld: warning: object file (/Users/main/Documents/code/ponyc/build/debug/libponyrt.a[10](fun.c.o)) was built for newer 'macOS' version (14.0) than being linked (13.0)
ld: warning: object file (/Users/main/Documents/code/ponyc/build/debug/libponyrt.a[11](hash.c.o)) was built for newer 'macOS' version (14.0) than being linked (13.0)
ld: warning: object file (/Users/main/Documents/code/ponyc/build/debug/libponyrt.a[13](stack.c.o)) was built for newer 'macOS' version (14.0) than being linked (13.0)
ld: warning: object file (/Users/main/Documents/code/ponyc/build/debug/libponyrt.a[14](actormap.c.o)) was built for newer 'macOS' version (14.0) than being linked (13.0)
ld: warning: object file (/Users/main/Documents/code/ponyc/build/debug/libponyrt.a[15](cycle.c.o)) was built for newer 'macOS' version (14.0) than being linked (13.0)
ld: warning: object file (/Users/main/Documents/code/ponyc/build/debug/libponyrt.a[16](delta.c.o)) was built for newer 'macOS' version (14.0) than being linked (13.0)
ld: warning: object file (/Users/main/Documents/code/ponyc/build/debug/libponyrt.a[17](gc.c.o)) was built for newer 'macOS' version (14.0) than being linked (13.0)
ld: warning: object file (/Users/main/Documents/code/ponyc/build/debug/libponyrt.a[18](objectmap.c.o)) was built for newer 'macOS' version (14.0) than being linked (13.0)
ld: warning: object file (/Users/main/Documents/code/ponyc/build/debug/libponyrt.a[19](serialise.c.o)) was built for newer 'macOS' version (14.0) than being linked (13.0)
ld: warning: object file (/Users/main/Documents/code/ponyc/build/debug/libponyrt.a[20](trace.c.o)) was built for newer 'macOS' version (14.0) than being linked (13.0)
ld: warning: object file (/Users/main/Documents/code/ponyc/build/debug/libponyrt.a[24](lsda.c.o)) was built for newer 'macOS' version (14.0) than being linked (13.0)
ld: warning: object file (/Users/main/Documents/code/ponyc/build/debug/libponyrt.a[26](posix_except.c.o)) was built for newer 'macOS' version (14.0) than being linked (13.0)
ld: warning: object file (/Users/main/Documents/code/ponyc/build/debug/libponyrt.a[28](socket.c.o)) was built for newer 'macOS' version (14.0) than being linked (13.0)
ld: warning: object file (/Users/main/Documents/code/ponyc/build/debug/libponyrt.a[31](stdfd.c.o)) was built for newer 'macOS' version (14.0) than being linked (13.0)
ld: warning: object file (/Users/main/Documents/code/ponyc/build/debug/libponyrt.a[34](alloc.c.o)) was built for newer 'macOS' version (14.0) than being linked (13.0)
ld: warning: object file (/Users/main/Documents/code/ponyc/build/debug/libponyrt.a[35](heap.c.o)) was built for newer 'macOS' version (14.0) than being linked (13.0)
ld: warning: object file (/Users/main/Documents/code/ponyc/build/debug/libponyrt.a[36](pagemap.c.o)) was built for newer 'macOS' version (14.0) than being linked (13.0)
ld: warning: object file (/Users/main/Documents/code/ponyc/build/debug/libponyrt.a[37](pool.c.o)) was built for newer 'macOS' version (14.0) than being linked (13.0)
ld: warning: object file (/Users/main/Documents/code/ponyc/build/debug/libponyrt.a[39](options.c.o)) was built for newer 'macOS' version (14.0) than being linked (13.0)
ld: warning: object file (/Users/main/Documents/code/ponyc/build/debug/libponyrt.a[40](ponyassert.c.o)) was built for newer 'macOS' version (14.0) than being linked (13.0)
ld: warning: object file (/Users/main/Documents/code/ponyc/build/debug/libponyrt.a[41](threads.c.o)) was built for newer 'macOS' version (14.0) than being linked (13.0)
ld: warning: object file (/Users/main/Documents/code/ponyc/build/debug/libponyrt.a[42](cpu.c.o)) was built for newer 'macOS' version (14.0) than being linked (13.0)
ld: warning: object file (/Users/main/Documents/code/ponyc/build/debug/libponyrt.a[43](mpmcq.c.o)) was built for newer 'macOS' version (14.0) than being linked (13.0)
ld: warning: object file (/Users/main/Documents/code/ponyc/build/debug/libponyrt.a[44](mutemap.c.o)) was built for newer 'macOS' version (14.0) than being linked (13.0)
ld: warning: object file (/Users/main/Documents/code/ponyc/build/debug/libponyrt.a[45](scheduler.c.o)) was built for newer 'macOS' version (14.0) than being linked (13.0)
ld: warning: object file (/Users/main/Documents/code/ponyc/build/debug/libponyrt.a[46](start.c.o)) was built for newer 'macOS' version (14.0) than being linked (13.0)

@d-led
Copy link

d-led commented Oct 24, 2023

same warnings as above, and make build works + make test link and run ok

@SeanTAllen
Copy link
Member Author

SeanTAllen commented Oct 25, 2023

@jemc can you help me try to figure out the proper flags to compile the runtime with so that it is for 13.0.0 and not for "whatever this version of the OS" is? That will get rid of the warnings. So that what we sent at pony program link time matches with what we set and compilation time.

I think ideally we want this to be a define that gets set and we can easily keep that up to date with what we use when we link via a define. So the "13.0.0" that is in our linking code is set to whatever the define was at the time the compiler and runtime were built.

For the cmake bits, I think we will probably lean on @chalcolith.

I believe we want to use https://cmake.org/cmake/help/latest/variable/CMAKE_OSX_DEPLOYMENT_TARGET.html at the time that we are building the runtime and then use the same version for linking.

I suggest setting to 13.0.0 for now.

For the sake of "libponyc-standalone" i think we should compile everything in src with the deployment option rather than just the libponyrt directory.

@jemc
Copy link
Member

jemc commented Oct 27, 2023

As you suspected, adding this snippet to the src/libponyrt/CMakeLists.txt takes away the warnings for linking the runtime objects:

set(CMAKE_OSX_DEPLOYMENT_TARGET 13.0.0)

However, there is still a similar warning for the user program object (in this case it was the ring example I was building):

ld: warning: object file (/Users/main/Documents/code/ponyc/ring.o) was built for newer 'macOS' version (14.0) than being linked (13.0)

I'm still trying to work out how we can set up the LLVM TargetMachine to have the specific MacOS platform version in genobj.c.

@SeanTAllen
Copy link
Member Author

Ah so 3 places. We definitely want a define to keep those straight and not garbled.

@jemc
Copy link
Member

jemc commented Oct 27, 2023

Here are the three (in total) places that we need to set the version (see the 13.0.0 value there):

diff --git a/src/libponyc/codegen/codegen.c b/src/libponyc/codegen/codegen.c
index c79df120..9a903670 100644
--- a/src/libponyc/codegen/codegen.c
+++ b/src/libponyc/codegen/codegen.c
@@ -818,9 +818,10 @@ bool codegen_pass_init(pass_opt_t* opt)
   {
     triple = LLVMCreateMessage(opt->triple);
   } else {
-#if defined(PLATFORM_IS_MACOSX) && !defined(PLATFORM_IS_ARM)
-    // This is to prevent XCode 7+ from claiming OSX 14.5 is required.
-    triple = LLVMCreateMessage("x86_64-apple-macosx");
+#if defined(PLATFORM_IS_MACOSX) && defined(PLATFORM_IS_ARM)
+    triple = LLVMCreateMessage("arm64-apple-macosx13.0.0");
+#elif defined(PLATFORM_IS_MACOSX) && !defined(PLATFORM_IS_ARM)
+    triple = LLVMCreateMessage("x86_64-apple-macosx13.0.0");
 #else
     triple = LLVMGetDefaultTargetTriple();
 #endif
diff --git a/src/libponyc/codegen/genexe.c b/src/libponyc/codegen/genexe.c
index 8b3af780..4d6299e3 100644
--- a/src/libponyc/codegen/genexe.c
+++ b/src/libponyc/codegen/genexe.c
@@ -295,7 +295,7 @@ static bool link_exe(compile_t* c, ast_t* program,
   snprintf(ld_cmd, ld_len,
     "%s -execute -arch %.*s "
     "-o %s %s %s %s "
-    "-L/Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/lib -lSystem %s",
+    "-L/Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/lib -lSystem %s -platform_version macos '13.0.0' '0.0.0'",
            linker, (int)arch_len, c->opt->triple, file_exe, file_o,
            lib_args, ponyrt, sanitizer_arg
     );
diff --git a/src/libponyrt/CMakeLists.txt b/src/libponyrt/CMakeLists.txt
index d9b6fb9b..f96a13ce 100644
--- a/src/libponyrt/CMakeLists.txt
+++ b/src/libponyrt/CMakeLists.txt
@@ -54,6 +54,8 @@ set(_c_src
 set(_ll_except_src "${CMAKE_CURRENT_SOURCE_DIR}/lang/except_try_catch.ll")
 set(_ll_except_obj "${CMAKE_BINARY_DIR}/except_try_catch.o")
 
+set(CMAKE_OSX_DEPLOYMENT_TARGET 13.0.0)
+
 find_file(_llc_command
     NAMES llc.exe llc
     PATHS "${CMAKE_BINARY_DIR}/../../libs/bin" "${CMAKE_BINARY_DIR}/../libs/bin"

@chalcolith - do you think you could help us get this 13.0.0 value moved to a single "define" in CMake?

@SeanTAllen SeanTAllen changed the title Fix compiling Pony programs on MacOS when XCode 15 is the linker Fix compiling Pony programs on MacOS when XCode 15 is the linker on x86_64 Oct 27, 2023
@SeanTAllen
Copy link
Member Author

I've added the changes that Joe and I figured out today to the PR + a change in the libponyc building as well.

There's probably a better way to set across all our CMakelists but maybe not.

@chalcolith can you review the changes in CMake plus make any updates needed to get the 13.0.0 into a define and not be hardcoded 5 places?

@SeanTAllen
Copy link
Member Author

I'll work on adding release notes to this.

@SeanTAllen SeanTAllen added the changelog - fixed Automatically add "Fixed" CHANGELOG entry on merge label Oct 27, 2023
@ponylang-main
Copy link
Contributor

Hi @SeanTAllen,

The changelog - fixed label was added to this pull request; all PRs with a changelog label need to have release notes included as part of the PR. If you haven't added release notes already, please do.

Release notes are added by creating a uniquely named file in the .release-notes directory. We suggest you call the file 4466.md to match the number of this pull request.

The basic format of the release notes (using markdown) should be:

## Title

End user description of changes, why it's important,
problems it solves etc.

If a breaking change, make sure to include 1 or more
examples what code would look like prior to this change
and how to update it to work after this change.

Thanks.

@SeanTAllen SeanTAllen requested a review from jemc October 27, 2023 22:08
@SeanTAllen SeanTAllen changed the title Fix compiling Pony programs on MacOS when XCode 15 is the linker on x86_64 Fix compiling Pony programs on X86 MacOS when XCode 15 is the linker Oct 28, 2023
@SeanTAllen
Copy link
Member Author

I've added release notes to this and once CI passes, I will be merging this and doing a release.

@SeanTAllen SeanTAllen marked this pull request as ready for review October 28, 2023 22:26
@d-led
Copy link

d-led commented Oct 28, 2023

sorry for late reply - llvm rebuild took a day. Build & test are good on my machine. Only, perhaps I've missed this one before:

/Library/Developer/CommandLineTools/usr/bin/libtool: warning renaming duplicate member name 'Trace.cpp.o' from '/Users/.../src/ponyc/src/libponyc/../../build/libs/lib/libLLVMAnalysis.a(Trace.cpp.o)' and '/Users/..../src/ponyc/src/libponyc/../../build/libs/lib/libLLVMXRay.a(Trace.cpp.o)'

several of those warnings but probably unrelated

@SeanTAllen
Copy link
Member Author

sorry for late reply - llvm rebuild took a day. Build & test are good on my machine. Only, perhaps I've missed this one before:

/Library/Developer/CommandLineTools/usr/bin/libtool: warning renaming duplicate member name 'Trace.cpp.o' from '/Users/.../src/ponyc/src/libponyc/../../build/libs/lib/libLLVMAnalysis.a(Trace.cpp.o)' and '/Users/..../src/ponyc/src/libponyc/../../build/libs/lib/libLLVMXRay.a(Trace.cpp.o)'

several of those warnings but probably unrelated

yup unrelated.

@SeanTAllen SeanTAllen merged commit cb653fa into main Oct 29, 2023
22 checks passed
@SeanTAllen SeanTAllen deleted the ldclassic branch October 29, 2023 01:09
@ponylang-main ponylang-main removed the discuss during sync Should be discussed during an upcoming sync label Oct 29, 2023
github-actions bot pushed a commit that referenced this pull request Oct 29, 2023
github-actions bot pushed a commit that referenced this pull request Oct 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog - fixed Automatically add "Fixed" CHANGELOG entry on merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants