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

compiler_rt: fix __clzsi2_thumb1 (incorrect lookup table usage) #22367

Merged
merged 1 commit into from
Jan 2, 2025

Conversation

GalaxyShard
Copy link
Contributor

Originally the code loaded the (first 4 bytes of the) lookup table into r3 instead of the address of the lookup table, resulting in the memory address 02020100 (the lookup table bytes, in little endian) being dereferenced instead of the address of the lookup table.

@GalaxyShard GalaxyShard changed the title compiler_rt: fix incorrect __clzsi2_thumb1 lookup table usage compiler_rt: fix __clzsi2_thumb1 (incorrect lookup table usage) Dec 30, 2024
@alexrp
Copy link
Member

alexrp commented Dec 31, 2024

A bit annoying that we apparently don't have test coverage of this. I'm half tempted to just delete this Thumb-1 specific code because, honestly, why?

Anyway, this is effectively a revert of #22051. We should avoid the use of the =.lut syntax.

@GalaxyShard
Copy link
Contributor Author

Anyway, this is effectively a revert of #22051. We should avoid the use of the =.lut syntax.

I'm not entirely sure how to fix this without causing #22050 again, unless the generic implementation is used instead of the thumb1 implementation. Using the generic one might have a performance impact though.

@alexrp
Copy link
Member

alexrp commented Jan 1, 2025

We would just need an extra load, no? Ah, no, ignore that. I see the problem now.

@alexrp alexrp enabled auto-merge (rebase) January 1, 2025 18:24
@alexrp alexrp merged commit e6879e9 into ziglang:master Jan 2, 2025
10 checks passed
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