Skip to content

Convert _mm_slli_epi{16,32,64} & _mm_srli_epi{16,32,64} to const generics #1020

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Feb 28, 2021
Merged

Conversation

tmiasko
Copy link
Contributor

@tmiasko tmiasko commented Feb 27, 2021

  • _mm_slli_epi64
  • _mm_slli_epi32
  • _mm_slli_epi16
  • _mm_srli_epi64
  • _mm_srli_epi32
  • _mm_srli_epi16

@rust-highfive
Copy link

r? @Amanieu

(rust-highfive has picked a reviewer for you, use r? to override)

@tmiasko tmiasko changed the title Convert intrinsics to const generics 1 of .. Convert intrinsics to const generics 1 Feb 27, 2021
@tmiasko tmiasko changed the title Convert intrinsics to const generics 1 Convert _mm_slli_epi{16,32,64} & _mm_srli_epi{16,32,64} to const generics Feb 27, 2021
}
constify_imm8!(imm8, call)
pub unsafe fn _mm_slli_epi16<const imm8: i32>(a: __m128i) -> __m128i {
transmute(pslliw(a.as_i16x8(), imm8))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should have a static_assert! to ensure the immediate is between 0 and 255.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In fact use static_assert_imm8 that was added in #1021.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added range checks using static_assert_imm8.

What do you use as a reference point, to make decision whether it should be hard error or not? The fact that it is documented as imm8? clang, gcc, and icc, as far as I can see accept arbitrary values there (in fact run-time ones as well).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We follow the intel intrinsics data which specifies that the argument is an 8-bit immediate. There is a separate intrinsic (_mm_sll_epi32) which allows run-time values.

<intrinsic tech="SSE2" vexEq="TRUE" name="_mm_slli_epi32">
	<type>Integer</type>
	<CPUID>SSE2</CPUID>
	<category>Shift</category>
	<return type="__m128i" varname="dst" etype="UI32"/>
	<parameter type="__m128i" varname="a" etype="UI32"/>
	<parameter type="int" varname="imm8" etype="IMM" immwidth="8"/>
	<description>Shift packed 32-bit integers in "a" left by "imm8" while shifting in zeros, and store the results in "dst".</description>
	<operation>
FOR j := 0 to 3
	i := j*32
	IF imm8[7:0] &gt; 31
		dst[i+31:i] := 0
	ELSE
		dst[i+31:i] := ZeroExtend32(a[i+31:i] &lt;&lt; imm8[7:0])
	FI
ENDFOR
	</operation>
	<instruction name="PSLLD" form="xmm, imm8" xed="PSLLD_XMMdq_IMMb"/>
	<header>emmintrin.h</header>
</intrinsic>

The presence of an appropriate static_assert! should probably be enforced by stdarch-verify, I'll look into implementing that.

@Amanieu Amanieu merged commit 283e6a8 into rust-lang:master Feb 28, 2021
@tmiasko tmiasko deleted the c1 branch February 28, 2021 16:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants