Stream: git-wasmtime

Topic: wasmtime / PR #5746 Cranelift: Generalize `(x << k) >> k`...


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

jameysharp opened PR #5746 from opts-shifts to main.

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

jameysharp requested cfallin for a review on PR #5746.

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

jameysharp submitted PR review.

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

jameysharp created PR review comment:

I'm a little confused about the requirements for iconst. Its documentation says it can create "an integer vector where all the lanes have the same value", which probably should work here. Do I just pass the lane type instead of the original ty in some of these places? ty_mask, at least, and also maybe imm64_ushr I guess?

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

cfallin submitted PR review.

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

cfallin created PR review comment:

Hmm -- that feature is somewhat surprising to me! I don't think we actually have lowerings that do that (and in fact I think we have another opcode for "vector broadcast" or whatever the proper name for that operation is)?

view this post on Zulip Wasmtime GitHub notifications bot (Feb 08 2023 at 18:19):

bjorn3 created PR review comment:

and in fact I think we have another opcode for "vector broadcast" or whatever the proper name for that operation is

It's called splat.

view this post on Zulip Wasmtime GitHub notifications bot (Feb 08 2023 at 18:20):

bjorn3 submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Feb 08 2023 at 23:48):

cfallin submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Feb 08 2023 at 23:48):

cfallin submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Feb 08 2023 at 23:48):

cfallin created PR review comment:

(same here as above)

view this post on Zulip Wasmtime GitHub notifications bot (Feb 08 2023 at 23:48):

cfallin created PR review comment:

(After talking with Jamey offline about this) it seems this is actually an equality constraint, counterintuitively! The narrow_mask is already defined and we start with the existing binding environment, not a fresh one, when building the AST for the LHS pattern; so we get the behavior one would for a second use of a variable in a pattern.

Here we should write an explicit equality (e.g. (if-let $true (u64_eq ...))) I think, and separately think about the above binding behavior...

view this post on Zulip Wasmtime GitHub notifications bot (Feb 08 2023 at 23:48):

cfallin created PR review comment:

This would be a little easier to parse if it were split across multiple lines, IMHO (just a preference!).

view this post on Zulip Wasmtime GitHub notifications bot (Feb 08 2023 at 23:48):

cfallin created PR review comment:

(same comment here re: equality test)

view this post on Zulip Wasmtime GitHub notifications bot (Feb 08 2023 at 23:48):

cfallin created PR review comment:

(same comment re: equality)

view this post on Zulip Wasmtime GitHub notifications bot (Feb 08 2023 at 23:48):

cfallin created PR review comment:

Just for clarity, even though it's strictly redundant given CLIF typechecking, could we write wide for the wildcards for the type params to ishl and sextend here? (General comment across the rules, and we may not have been consistent elsewhere with this; but especially for rules operating across multiple types, it makes it much easier to reason about!)

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

jameysharp updated PR #5746 from opts-shifts to main.

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

jameysharp updated PR #5746 from opts-shifts to main.

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

jameysharp updated PR #5746 from opts-shifts to main.

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

cfallin submitted PR review.

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

jameysharp merged PR #5746.


Last updated: Dec 23 2024 at 12:05 UTC