Skip to content
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

x86 __m128, __m256, __m512 rename / retype ? #214

Closed
gnzlbg opened this issue Nov 22, 2017 · 7 comments · Fixed by #232
Closed

x86 __m128, __m256, __m512 rename / retype ? #214

gnzlbg opened this issue Nov 22, 2017 · 7 comments · Fixed by #232

Comments

@gnzlbg
Copy link
Contributor

gnzlbg commented Nov 22, 2017

Currently the _m128i and _m256i are just type aliases for i8x16 and i8x32:

/// 128-bit wide signed integer vector type
#[allow(non_camel_case_types)]
pub type __m128i = ::v128::i8x16;
/// 256-bit wide signed integer vector type
#[allow(non_camel_case_types)]
pub type __m256i = ::v256::i8x32;

These are the places that are currently using them (unless my grep-fu failed, please correct me if I am missing some):

  • Comparing strings where the input is reinterpreted as 1byte.8bytes, ... depending on a flag:

    • _mm_cmpistrm: Compare packed strings
    • _mm_cmpestri: Compare packed strings
    • ... most of the intrinsics in the sse42 module.
  • Shift bytes:

    • _mm_slli_si128: Shift a left by imm8 bytes while shifting in zeros.
    • _mm_bslli_si128: Shift a left by imm8 bytes while shifting in zeros.
    • _mm_bsrli_si128: Shift a right by imm8 bytes while shifting in zeros.
    • _mm_srli_si128: Shift a right by imm8 bytes while shifting in zeros.
  • As raw bits i1x128 / i1x256:

    • _mm_testz_si128: Tests whether the specified bits in a 128-bit integer vector are all zeros.
    • _mm256_and_si256: Compute the bitwise AND of 256 bits (representing integer data) in a and b.
    • _mm256_extractf128_si256: Extract 128 bits (composed of integer data) from a, selected with imm8.
    • _mm_lddqu_si128: Load 128-bits of integer data from unaligned memory.

Switching __mm_cmpistrm to use bytes explicitly, and rename __m128i/__m256i to i1x128/i1x256 might make the library more consistent. I am not sure. I also haven't used __mm_cmpistrm before so maybe I am wrong to think that bytes there make sense. It makes sense for these functions to operate on generic __m128i types because how these are interpreted by the function depends on the function arguments passed. The shift byte functions should, however, operate on bytes.

Whether we can implement an i1x128 repr(simd) type in current Rust is a different topic (LLVM supports this, but we might need to "tweak" rustc to make it work).

Thoughts?


This discussion spawned in #212 .

@gnzlbg gnzlbg changed the title x86 __m128, __m256, __m512 x86 __m128, __m256, __m512 rename / retype ? Nov 22, 2017
@BurntSushi
Copy link
Member

BurntSushi commented Nov 22, 2017

IIRC, the motivation for defining these types was mostly as a way to express the vector type for intrinsics that don't really operate using any notion of lane width, and instead treat the vector as just a bunch of bits. I don't really know what the right choice is. I suppose a special i1x128 type would be the best, but I don't think we actually need compiler support for that to make a decision, right? I think these are our choices from the perspective of the public API:

  • Don't define any aliases. For intrinsics that accept vectors as a bundle of bits, pick an arbitrary integer vector type (say, i8x16) and just use that.
  • Define an alias to an arbitrary integer vector type. (The status quo.)
  • Define a new type called i1x128. (Internally, it could be any arbitrary integer vector type or a theoretical i1x128 vector if and when rustc gets support for it.)

To a first approximation, any intrinsic ending in si{128,256} uses this type.

@gnzlbg
Copy link
Contributor Author

gnzlbg commented Nov 22, 2017

It occurs to me that a problem with the type alias approach is that they get the operators from the alias, but the i8x32 operators might not make much sense for __m256i.

Not using a type alias (independently of whether we just pick a concrete type, or create or own) would fix that.


I thought a couple of times about just using Rust's i128 type here but I don't know if this would work. That is, if we could reinterpret it as a vector type.

That won't work for 256bit and 512bit though, and it will be another thing that those core users that don't have 128-bit support yet will need to work around.

@gnzlbg
Copy link
Contributor Author

gnzlbg commented Nov 22, 2017

FWIW using i128 does work but... I think I don't like it.

@gnzlbg
Copy link
Contributor Author

gnzlbg commented Nov 28, 2017

Note the __m64 type must be public (it is exposed by some MMX intrinsics). Also __m64 is not an llvm vector type, but an llvm x86_mmx type, so we can't just simply define it using a type alias.

@gnzlbg
Copy link
Contributor Author

gnzlbg commented Nov 28, 2017

@alexcrichton do you know if one can use x86_mmx on other architectures, like ARM ? If not, then that might be a non-portable vector type (as in, we might need to do something different on ARM to be able to use it).

@alexcrichton
Copy link
Member

Nah it is not usable but rustc also won't generate it on arm

@gnzlbg
Copy link
Contributor Author

gnzlbg commented Dec 12, 2017

@BurntSushi I've defined the new types in #232 and updated the tests to use them, but I haven't renamed them to i1x{64,128,256} yet (I'd rather do that in a different PR).

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 a pull request may close this issue.

3 participants