Stream: git-cranelift

Topic: cranelift / PR #1371 Add rounding average instructions fo...


view this post on Zulip GitHub (Jan 31 2020 at 22:42):

abrown opened PR #1371 from rounding-average to master:

view this post on Zulip GitHub (Jan 31 2020 at 22:42):

abrown requested bnjbvr for a review on PR #1371.

view this post on Zulip GitHub (Feb 21 2020 at 16:03):

bnjbvr submitted PR Review.

view this post on Zulip GitHub (Feb 21 2020 at 16:03):

bnjbvr submitted PR Review.

view this post on Zulip GitHub (Feb 21 2020 at 16:03):

bnjbvr created PR Review Comment:

Can the parser understand the hexadecimal syntax? If so, we should use it here, and then we wouldn't need the comment anymore.

view this post on Zulip GitHub (Feb 21 2020 at 16:03):

bnjbvr created PR Review Comment:

nit: byte => word

view this post on Zulip GitHub (Feb 21 2020 at 16:03):

bnjbvr created PR Review Comment:

Should we use the more current abbreviation "avg", or just be explicit and call it "average" (it's not very long)? This is the first time I see this midway abbreviation for average, but I'm not the English native here.

view this post on Zulip GitHub (Feb 21 2020 at 17:12):

bjorn3 submitted PR Review.

view this post on Zulip GitHub (Feb 21 2020 at 17:12):

bjorn3 created PR Review Comment:

I believe a word is 16 bits, not 8.

view this post on Zulip GitHub (Feb 21 2020 at 17:12):

bjorn3 deleted PR Review Comment.

view this post on Zulip GitHub (Feb 21 2020 at 18:32):

abrown submitted PR Review.

view this post on Zulip GitHub (Feb 21 2020 at 18:32):

abrown created PR Review Comment:

The complication here is that the semantics are actually "rounding average" instead of just "average"; I went along with the term ["avgr" used by the spec](fhttps://github.com/WebAssembly/simd/blob/master/proposals/simd/SIMD.md#lane-wise-integer-minimum). Do you want to expand it to "rounding_average" or "average_round"?

view this post on Zulip GitHub (Feb 21 2020 at 18:32):

abrown edited PR Review Comment.

view this post on Zulip GitHub (Feb 21 2020 at 18:45):

abrown created PR Review Comment:

No, not yet for i16, i32, and i64; here's an issue to fix that: https://github.com/bytecodealliance/cranelift/issues/1401.

view this post on Zulip GitHub (Feb 21 2020 at 18:45):

abrown submitted PR Review.

view this post on Zulip GitHub (Feb 21 2020 at 21:46):

abrown updated PR #1371 from rounding-average to master:

view this post on Zulip GitHub (Feb 24 2020 at 09:13):

bnjbvr submitted PR Review.

view this post on Zulip GitHub (Feb 24 2020 at 09:13):

bnjbvr created PR Review Comment:

Good point! Maybe then avg_round is a nice tradeoff? "avg" is standard for average, and we make the rounding part very explicit.

view this post on Zulip GitHub (Feb 24 2020 at 17:12):

abrown updated PR #1371 from rounding-average to master:

view this post on Zulip GitHub (Feb 24 2020 at 17:14):

abrown updated PR #1371 from rounding-average to master:

view this post on Zulip GitHub (Feb 24 2020 at 17:32):

abrown updated PR #1371 from rounding-average to master:

view this post on Zulip GitHub (Feb 24 2020 at 17:48):

abrown merged PR #1371.


Last updated: Nov 22 2024 at 16:03 UTC