Stream: git-cranelift

Topic: cranelift / PR #1376 Add integer min/max instructions for...

GitHub (Feb 04 2020 at 17:33):

abrown opened PR #1376 from integer-min-max to master:

• [x] This has not been discussed in an issue.

• [x] A short description of what this does, why it is needed:
  The Wasm SIMD spec added integer min/max instructions. For x86 codegen, Cranelift already had encodings for the instructions to emit (e.g. x86_pmins) but did not have shared instructions with which to translate the Wasm integer min/max. This change adds [i|u]min and [i|u]max to do just that and legalizes these new Cranelift instructions to the pre-existing x86 instructions. At some point we could remove the x86 instructions entirely and attach their encodings directly on [i|u]min and [i|u]max.

• [x] This PR contains test cases, if meaningful.

• [x] A reviewer from the core maintainer team has been assigned for this PR.

<!-- Please ensure all communication adheres to the code of conduct. -->

GitHub (Feb 04 2020 at 17:33):

abrown requested bnjbvr for a review on PR #1376.

GitHub (Feb 21 2020 at 16:12):

bnjbvr submitted PR Review.

GitHub (Feb 21 2020 at 16:12):

bnjbvr submitted PR Review.

GitHub (Feb 21 2020 at 16:12):

bnjbvr created PR Review Comment:

Forgot to tell in the previous review for average (and if you could apply it there too that'd be great), but as a way to make sure we stop regressing build-step compile time, can you put these in a different function marked as #[inline(never)] please? (No need to move all the existing instructions.) This for instance could go into a simd_comparisons function, or something appropriately named like this. Thanks!

GitHub (Feb 21 2020 at 17:20):

abrown updated PR #1376 from integer-min-max to master:

• [x] This has not been discussed in an issue.

• [x] A short description of what this does, why it is needed:
  The Wasm SIMD spec added integer min/max instructions. For x86 codegen, Cranelift already had encodings for the instructions to emit (e.g. x86_pmins) but did not have shared instructions with which to translate the Wasm integer min/max. This change adds [i|u]min and [i|u]max to do just that and legalizes these new Cranelift instructions to the pre-existing x86 instructions. At some point we could remove the x86 instructions entirely and attach their encodings directly on [i|u]min and [i|u]max.

• [x] This PR contains test cases, if meaningful.

• [x] A reviewer from the core maintainer team has been assigned for this PR.

<!-- Please ensure all communication adheres to the code of conduct. -->

GitHub (Feb 21 2020 at 17:21):

abrown submitted PR Review.

GitHub (Feb 21 2020 at 17:21):

abrown created PR Review Comment:

I went with define_simd_arithmetic since it seems to be in that section in the spec.

GitHub (Feb 21 2020 at 17:33):

abrown merged PR #1376.

Last updated: Dec 23 2024 at 14:03 UTC