-
-
Notifications
You must be signed in to change notification settings - Fork 212
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
Add initial MacOS ARM support #7073
Conversation
@@ -222,6 +222,10 @@ | |||
#ifdef ARM | |||
#define FB_CPU CpuArm | |||
#endif /* ARM */ | |||
#ifdef ARM64 | |||
#define DARWIN64 |
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.
It does not seems correct to use DARWIN64
(same as used for x86_64) here.
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.
You need to use new constant here (DARWINARM64?) and adjust src/remote/remote_def.h, src/remote/protocol.h and src/remote/inet.cpp with arch_darwin_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.
It would also need changes in src/common/classes/DbImplementation.cpp.
Ww would better wait for @AlexPeshkoff feedback :)
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.
Certainly, using different constants for different CPu like you suggest will for sure work. And I think that right now in order to have port better sooner it's right choice.
But longterm we should better understand why are different ARCHITECTUREs needed for different CPU under MacOS. Currently they are ORed in inet.cpp. Is it to avoid symmetric protocol between them? If yes, why is it not needed under linux with different CPU?
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.
Looks like currently there is no need separating Darwin's ARCHITECTURE per HW. I.e. using DARWIN64 is OK for arm64.
✅ Build firebird 1.0.3752 completed (commit 46e8f643c9 by @e787) |
✅ Build firebird 1.0.3768 completed (commit e6a87a3a7f by @e787) |
Files created/edited to be able to compile project under M1 processors