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 arm64 MacOS builds. #4

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

d4rk
Copy link

@d4rk d4rk commented Nov 4, 2024

Main changes are:

  1. Added an ARM64 assembly version of ft_strsplit.s since the x86 ASM doesn't work on ARM64.
  2. Updated Makefile to use it on arm64 architectures.
  3. Changed the type of t_bool to unsigned to avoid the single-bit-bitfield-constant-conversion warning.

@stephen-gardner
Copy link
Owner

stephen-gardner commented Nov 4, 2024

Thank you for the pull request, @d4rk. Please note that the ft_strsplit used in this project differs from the libft project spec. As seen here and here, the ft_strsplit used in this project does a single memory allocation instead of many.

@d4rk
Copy link
Author

d4rk commented Nov 5, 2024

Thank you for the pull request, @d4rk. Please note that the ft_strsplit used in this project differs from the libft project spec. As seen here and here, the ft_strsplit used in this project does a single memory allocation instead of many.

Got it, thanks for the clarification. I'll try to address this some time this week.

@d4rk d4rk force-pushed the fix-mac-build branch 3 times, most recently from 9bef1db to 6b1c020 Compare November 8, 2024 02:58
@stephen-gardner
Copy link
Owner

Hi @d4rk, thanks for the revisions! Unfortunately, I don't have a test environment for ARM64 architecture, except on my work machine. It could be some time before I can properly review the change. Is there a particular reason why you chose to write this function in assembly instead of C?

@d4rk
Copy link
Author

d4rk commented Nov 8, 2024

Hi @d4rk, thanks for the revisions! Unfortunately, I don't have a test environment for ARM64 architecture, except on my work machine. It could be some time before I can properly review the change.

No worries at all.

Is there a particular reason why you chose to write this function in assembly instead of C?

Not really, I thought it might be fun :)

Main changes are:
1. Introduce an AARCH64 assembly version of `ft_strsplit.s`, similar to the x86_64 version.
2. Change the type of `t_bool` to `unsigned` to avoid the `single-bit-bitfield-constant-conversion` warning.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants