-
Notifications
You must be signed in to change notification settings - Fork 319
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
Added support for Apple Silicon (M1). #273
Conversation
Noted. We'll look into it. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These lines need properly indenting to match surrounding #if blocks
groups/bsl/bsls/bsls_spinlock.h
Outdated
@@ -292,7 +292,7 @@ extern "C" { | |||
#include <time.h> | |||
#endif | |||
|
|||
#if !(defined(BSLS_PLATFORM_OS_SOLARIS)) && !(defined(BSLS_PLATFORM_OS_AIX)) | |||
#if !(defined(BSLS_PLATFORM_OS_SOLARIS)) && !(defined(BSLS_PLATFORM_OS_AIX)) && !(defined(__arm64)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wordwrap with a line continuation for exceeding 79 columns
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We shouldn't reference system macros directly but should be checking for the proper BSLS_PLATFORM macros.
Please change this (and the other preprocessor check below) to this:
#if !(defined(BSLS_PLATFORM_OS_SOLARIS)) \
&& !(defined(BSLS_PLATFORM_OS_AIX)) \
&& !(defined(BSLS_PLATFORM_OS_DARWIN) && defined(BSLS_PLATFORM_CPU_ARM))
groups/bsl/bsls/bsls_spinlock.h
Outdated
@@ -451,7 +451,7 @@ void SpinLock::doBackoff(int *count) { | |||
|
|||
inline | |||
void SpinLock::pause() { | |||
#if !(defined(BSLS_PLATFORM_OS_SOLARIS)) && !(defined(BSLS_PLATFORM_OS_AIX)) | |||
#if !(defined(BSLS_PLATFORM_OS_SOLARIS)) && !(defined(BSLS_PLATFORM_OS_AIX)) && !(defined(__arm64)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wordwrap with a line continuation for exceeding 79 columns
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please change the macro guards in bsls_spinlock as I specified and then verify that this still builds on your M1. If it does we can get this PR landed.
Thanks.
groups/bsl/bsls/bsls_spinlock.h
Outdated
@@ -292,7 +292,7 @@ extern "C" { | |||
#include <time.h> | |||
#endif | |||
|
|||
#if !(defined(BSLS_PLATFORM_OS_SOLARIS)) && !(defined(BSLS_PLATFORM_OS_AIX)) | |||
#if !(defined(BSLS_PLATFORM_OS_SOLARIS)) && !(defined(BSLS_PLATFORM_OS_AIX)) && !(defined(__arm64)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We shouldn't reference system macros directly but should be checking for the proper BSLS_PLATFORM macros.
Please change this (and the other preprocessor check below) to this:
#if !(defined(BSLS_PLATFORM_OS_SOLARIS)) \
&& !(defined(BSLS_PLATFORM_OS_AIX)) \
&& !(defined(BSLS_PLATFORM_OS_DARWIN) && defined(BSLS_PLATFORM_CPU_ARM))
Should this PR be closed in preference to #275? |
That does look like a more comprehensive PR than this one (I was just about to test). |
I updated to a more recent BDE and it no longer builds on the M1. I can work on it but if the other PR already works for a more recent version of BDE, it'd make more sense to merge that one. Let me know. |
Describe your changes
The existing compiler flags did not allow builds on Apple Silicon platforms (e.g., the M1 mini). This PR rectifies this situation, adding
__arm64
detection and ensuring it is treated as a 64-bit platform.Testing performed
Run with a test that uses the BDE library.