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

CBE: implement mod, divFloor, divTrunc, and enable more passing tests #11229

Merged
merged 1 commit into from
Mar 20, 2022

Conversation

jmc-88
Copy link
Contributor

@jmc-88 jmc-88 commented Mar 19, 2022

Also enables the newly-passing math.zig tests, while splitting off the half-precision floating point tests until #10771 is merged.

src/link/C/zig.h Outdated
Comment on lines 772 to 775
#define zig_div_trunc_int(ZigType, CType) \
static inline CType zig_div_trunc_##ZigType(CType numerator, CType denominator) { \
return numerator / denominator; \
}
Copy link
Member

Choose a reason for hiding this comment

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

If this is how div trunc for integers is defined then why don't we simply emit a / b instead of a call to this function? Looks like before this change, that's exactly what we did. Was there a problem with it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Very true, this was only convenient for a unified interface but we shouldn't need this code at all.

I changed the code in genBody() to instead selectively dispatch to airBinOp(..., " / ") or airBinOpBuiltinCall(..., "div_trunc") depending on the type, PTAL.

@doppioandante
Copy link

When I did some work in this I had added some more tests, maybe they could be useful (unfortunately I don't remember if I did for a specific reason lol).

@andrewrk andrewrk merged commit b6203b8 into ziglang:master Mar 20, 2022
@jmc-88 jmc-88 deleted the cbe branch March 20, 2022 08:06
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.

3 participants