-
Notifications
You must be signed in to change notification settings - Fork 171
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 missing __builtin_clz definition for MSVC #1808
Add missing __builtin_clz definition for MSVC #1808
Conversation
ae5d3cb
to
214c3fb
Compare
Huh, anybody have an idea what's happening here? https://dev.azure.com/MoarVM/MoarVM/_build/results?buildId=1423&view=logs&j=97945c8c-7ab8-5ec2-f2c7-7c8541d3bf5c&t=439f2dd7-104a-517a-a6b1-3f392bb69fda&l=2266 |
2fcfe0b
to
9e3ee7f
Compare
do you know that the name exists in the dll for sure? maybe something changed somewhere. you should be able to use |
I think the nativecall errors are unrelated to this change. But if you can explain https://dev.azure.com/MoarVM/MoarVM/_build/results?buildId=1436&view=logs&j=d36c29d8-9236-5cd5-d848-768a0b8a3eb3&t=d8ee4cfd-785a-54f9-bf7e-80664ee220b4&l=34 I would most grateful. If I pull our i64toa_jeaiii out into a separate file and run it with 1449334664 it's just fine. Is this a windows-only problem? |
And if I build this branch locally (with all the extra checks and debug fprintfs) it builds just fine without printing anything. |
The native call thing is tracked over in: rakudo/rakudo#5587, I'm pretty sure that's a different issue. |
Well, that definitely needs to be clarified, but it shouldn't be the cause. That's just to figure out how big of a string to allocate, but even writing into a definitely-big-enough stack-allocated string initialized to all 0 ends up wrong. |
not sure if this does anything, but try |
yeah at least some of the numbers in the debug output are suspiciously 31 bits long and then a much longer number for the output of m and m2 |
Maybe this could be related. After finding some test failures in What i found was, the tests sometime cause I mentioned my observations with a bit more detail here. |
This is probably helpful information, thanks. I wonder if we can easily add 32-bit Alpine Linux to our CI pipeline? |
Alpine itself uses Gitlab for CI, which is where i first noticed this issue, while trying to upgrade Rakudo and its dependencies. As for Github, an Alpine developer has this project which you may find helpful: https://github.com/jirutka/setup-alpine |
e8471b2
to
7fa2c51
Compare
@Cellesti the Windows CI jobs are passing now. Are you able to test with this branch? If so, and you tell me it's good, I'll cleanup all the commits and merge. |
@MasterDuke17 i have been working on a similar patch and just got it committed: https://git.alpinelinux.org/aports/tree/community/moarvm/coerce-invalid-array-access.patch With that patch, NQP and Rakudo tests are passing for all 8 architectures currently supported by Alpine. Let me know if there's anything else in this branch (besides adding another |
Nope, I think the rest was for Windows. Thanks, seems like this is probably good to go. |
You're welcome |
It was missed when copying #defines from https://github.com/llvm/llvm-project/blob/main/compiler-rt/lib/builtins/int_lib.h.
324a963
to
a7b6e3c
Compare
It was missed when copying #defines from
https://github.com/llvm/llvm-project/blob/main/compiler-rt/lib/builtins/int_lib.h.