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

mingw.zig: fix logic to add crt sources #9558

Merged
merged 1 commit into from
Aug 21, 2021

Conversation

marler8997
Copy link
Contributor

@marler8997 marler8997 commented Aug 13, 2021

The current version of code uses isARM to check if we are compiling to any arm target then checks the target bit width to either add the 32-bit sources or 64-bit source. However, isARM only returns true for 32-bit targets, and isAARCH64 is for the 64-bit targets.

I also replaced the unreachable with a @Panic when we receive an unsupported arch because this code is reachable and should turn into an error. With the current unreachable statement, the released compiler on windows was silently hitting the unreachable and falling through to the isARM case.

This is one of the errors contributing to #9519

It would be great if I could also add a regression test for this, not sure how though...any guidance here would be very helpful.

The current version of code uses isARM to check if we are compiling to any arm target then checks the target bit width to either add the 32-bit sources or 64-bit source.  However, isARM only returns true for 32-bit targets, and isAARCH64 is for the 64-bit targets.

I also replaced the unreachable with a @Panic when we receive an unsupported arch because this code is reachable and should turn into an error.
Copy link
Member

@Vexu Vexu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

std.Target.Cpu.Arch.isAARCH64() does not exist.

@Vexu Vexu added the frontend Tokenization, parsing, AstGen, Sema, and Liveness. label Aug 21, 2021
@marler8997
Copy link
Contributor Author

std.Target.Cpu.Arch.isAARCH64() does not exist.

Not sure what you're saying. Your casing is incorrect, it's supposed to be target.cpu.arch.isAARCH64(), and if it didn't exist then zig1.o wouldn't compile but it does, even if I rebase this branch wth master.

@Vexu
Copy link
Member

Vexu commented Aug 21, 2021

Huh, I have no idea where I was looking but I couldn't manage to find it earlier.

@Vexu Vexu merged commit f28868e into ziglang:master Aug 21, 2021
@marler8997 marler8997 deleted the fixMingwAarch64 branch August 21, 2021 17:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
frontend Tokenization, parsing, AstGen, Sema, and Liveness.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants