Skip to content

Implement @mod and fix bugs with divFloor for wasm#16899

Merged
Luukdegram merged 2 commits intoziglang:masterfrom
riverbl:wasm-div
Aug 24, 2023
Merged

Implement @mod and fix bugs with divFloor for wasm#16899
Luukdegram merged 2 commits intoziglang:masterfrom
riverbl:wasm-div

Conversation

@riverbl
Copy link
Contributor

@riverbl riverbl commented Aug 20, 2023

Implement lowering code for @mod on integers in the stage2 wasm backend (@mod on floats remains unimplemented).

Fix invalid wasm being produced for @divFloor on signed integers by the stage2 wasm backend. wasm comparison instructions (e.g., less than) always output an i32, and select takes an i32 as the condition, but the previous @divFloor implementation assumed that when comparing or selecting between i64, the output / condition would be an i64.

This pull request also renames signAbsValue in wasm/CodeGen.zig to signExtendInt, as I found the previous name misleading.

The new @divFloor implementation 0 extends its result when used on type iN where N < 64, N != 32 - see #16807 (comment).

@andrewrk andrewrk requested a review from Luukdegram August 21, 2023 18:23
Copy link
Contributor

@Luukdegram Luukdegram left a comment

Choose a reason for hiding this comment

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

Thank you for your contribution. I'd like to see a behavior test enabled that triggers this instruction as part of this PR before accepting. The rest is just some minor aesthetic changes, which aren't necessarily blocking this PR from being merged.

Implement lowering code for `@mod` on integers in the stage2 wasm backend

Fix invalid wasm being produced for `@divFloor` on signed integers by the stage2 wasm backend
Replace `@panic` with `unreachable` in stage2 wasm `@divFloor` implementation

Add test for division and remainder operations for stage2 wasm
@riverbl
Copy link
Contributor Author

riverbl commented Aug 23, 2023

I've added a test (copied from behavior/int_div.zig, plus some float test cases) covering @divFloor and @mod. It's only enabled for the stage2 wasm backend, and has a TODO comment advising that it be deleted once the division tests in behavior/int_div.zig and behavior/math.zig pass with the stage2 wasm backend.

@riverbl riverbl requested a review from Luukdegram August 23, 2023 19:47
@Luukdegram
Copy link
Contributor

Thank you, great work!

@Luukdegram Luukdegram merged commit dd6a9ca into ziglang:master Aug 24, 2023
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