Stream: git-wasmtime

Topic: wasmtime / PR #8546 riscv64: Move Round Instructions to ISLE

Wasmtime GitHub notifications bot (May 03 2024 at 19:20):

afonso360 requested cfallin for a review on PR #8546.

Wasmtime GitHub notifications bot (May 03 2024 at 19:20):

afonso360 requested wasmtime-compiler-reviewers for a review on PR #8546.

Wasmtime GitHub notifications bot (May 03 2024 at 19:20):

afonso360 opened PR #8546 from afonso360:riscv-isle-round to bytecodealliance:main:

:wave: Hey,

This PR is a small cleanup to our lowerings for rounding instructions (ceil/floor/trunc/nearest). It moves the generated code from a single pseudo-instruction into an ISLE lowering.

The rounding lowering remains mostly the same, except it removes an extraneous fadd that we ended up not really needing. It also improves the constant codegen since we have a more advanced algorithm in ISLE that does better pattern matching.

This PR also moves the ISA tests to separate files, but that is done on the first commit, so looking at the second commit individually gives the actual changes to the assembly output.

Wasmtime GitHub notifications bot (May 03 2024 at 19:24):

afonso360 edited PR #8546:

:wave: Hey,

This PR is a small cleanup to our lowerings for rounding instructions (ceil/floor/trunc/nearest). It moves the generated code from a single pseudo-instruction into an ISLE lowering.

The rounding lowering remains mostly the same, except it removes an extraneous fadd that we ended up not really needing. It also improves the constant codegen since we have a more advanced algorithm in ISLE that does better pattern matching.

This PR also moves the ISA tests to separate files, but that is done on the first commit, so looking at the second commit individually gives the actual changes to the assembly output.

Also, FloatRound was the only instruction in the riscv64_worst_case_instruction_size but the worst_case_size was way larger than what was actually needed for that instruction. I suspect there is some other instruction that may need that much size. I added a few cases, but none ended up at the 124 bytes that it is currently set to. Something to track down later...

Wasmtime GitHub notifications bot (May 03 2024 at 19:24):

afonso360 edited PR #8546:

:wave: Hey,

This PR is a small cleanup to our lowerings for rounding instructions (ceil/floor/trunc/nearest). It moves the generated code from a single pseudo-instruction into an ISLE lowering.

The rounding lowering remains mostly the same, except it removes an extraneous fadd that we ended up not really needing. It also improves the constant codegen since we have a more advanced algorithm in ISLE that does better pattern matching.

This PR also moves the ISA tests to separate files, but that is done on the first commit, so looking at the second commit individually gives the actual changes to the assembly output.

Also, FloatRound was the only instruction in the riscv64_worst_case_instruction_size test but the worst_case_size was way larger than what was actually needed for that instruction. I suspect there is some other instruction that may need that much size. I added a few cases, but none ended up at the 124 bytes that it is currently set to. Something to track down later...

Wasmtime GitHub notifications bot (May 03 2024 at 19:43):

alexcrichton submitted PR review:

Very nice refactoring :+1:

Wasmtime GitHub notifications bot (May 04 2024 at 10:02):

afonso360 updated PR #8546.

Wasmtime GitHub notifications bot (May 04 2024 at 10:16):

afonso360 updated PR #8546.

Wasmtime GitHub notifications bot (May 04 2024 at 17:23):

afonso360 merged PR #8546.

Last updated: Nov 22 2024 at 17:03 UTC