Stream: git-wasmtime

Topic: wasmtime / PR #5874 riscv64: Move `is_null`/`is_invalid` ...

view this post on Zulip Wasmtime GitHub notifications bot (Feb 24 2023 at 13:24):

afonso360 opened PR #5874 from riscv64-ref-check to main:

:wave: Hey,

Small PR moving is_null/is_invalid to ISLE. It also removes a few branches from the generated code.

It also fixes is_invalid which was slightly broken.

view this post on Zulip Wasmtime GitHub notifications bot (Feb 24 2023 at 17:56):

jameysharp submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Feb 24 2023 at 17:56):

jameysharp submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Feb 24 2023 at 17:56):

jameysharp created PR review comment:

I'd be inclined to not shadow v here with a new definition. It's not a bug or anything, but I had to read a little more carefully to see that the seqz consumes the redefinition, not the original input.

I think you could inline it, actually, and maybe that's more clear? I don't know, I haven't developed strong opinions about style in ISLE source yet.

  (seqz (alu_rr_imm12 (AluOPRRI.Addi) v (imm12_const 1))))

view this post on Zulip Wasmtime GitHub notifications bot (Feb 25 2023 at 11:37):

afonso360 updated PR #5874 from riscv64-ref-check to main.

view this post on Zulip Wasmtime GitHub notifications bot (Feb 25 2023 at 11:42):

afonso360 submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Feb 25 2023 at 11:42):

afonso360 created PR review comment:

I think you could inline it, actually, and maybe that's more clear? I don't know, I haven't developed strong opinions about style in ISLE source yet.

I haven't either, but it would be nice to have some sort of consensus among backends!

One of the things I noticed while working in the RISC-V backend is that we don't seem to define too many instructions helpers. We always do something like (alu_rrr (opcode) ...), I'm personally not a big fan of this, but I'm not sure how other people feel about this stuff.

Also maybe we could get some sort of lisp formatter that would work for folks? Styles are different across all backends, It'd be nice to standardize on something.

Maybe that is something for https://github.com/bytecodealliance/wasmtime/issues/5752, I'm not sure how far LSP goes.

view this post on Zulip Wasmtime GitHub notifications bot (Feb 25 2023 at 11:44):

afonso360 edited PR review comment.

view this post on Zulip Wasmtime GitHub notifications bot (Feb 25 2023 at 13:26):

afonso360 merged PR #5874.

Last updated: Nov 22 2024 at 17:03 UTC