Skip to content

Difference in SIMD semantics breaks translate-c code #23536

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

Open
Majora320 opened this issue Apr 11, 2025 · 4 comments
Open

Difference in SIMD semantics breaks translate-c code #23536

Majora320 opened this issue Apr 11, 2025 · 4 comments
Labels
bug Observed behavior contradicts documented or intended behavior translate-c C to Zig source translation feature (@cImport)

Comments

@Majora320
Copy link

Majora320 commented Apr 11, 2025

Zig Version

0.14.0

Steps to Reproduce and Observed Behavior

This bug occurs when translating C SIMD functions that use equality or comparison operators to Zig. For an example, here is a function from emmintrin.h:

static __inline__ __m128i __DEFAULT_FN_ATTRS _mm_cmpeq_epi8(__m128i __a,
                                                            __m128i __b) {
  return (__m128i)((__v16qi)__a == (__v16qi)__b);
}

Clang's vector extension semantics translate this into a single pcmpeqb instruction that writes the result of the comparison - which is the same size as the two 128-bit arguments - into the (inlined) result. translate-c translates this into:

pub inline fn _mm_cmpeq_epi8(arg___a: __m128i, arg___b: __m128i) __m128i {
    var __a = arg___a;
    _ = &__a;
    var __b = arg___b;
    _ = &__b;
    return @as(__m128i, @bitCast(@as(__v16qi, @bitCast(__a)) == @as(__v16qi, @bitCast(__b))));
}

This is pretty much a one-to-one translation of the C, but Zig's semantics around vector comparisons are different - Zig seems to perform a similar vpcmpeqb, but then performs a vpmovmskb instruction to extract the significant bits into a packed format. This results in a compilation error as the packed type produced by Zig, @Vector(16, bool), does not match the 128-bit source type.

Expected Behavior

Either the C translation code for SIMD comparisons should be modified to translate the packed 16-bit format back into the source format, or (my preference) Zig's SIMD semantics should be changed to always output comparisons (including < and >) in the source format. This would offer the programmer more flexibility and control in high-performance applications and fit more closely with the behavior of other languages. The programmer can manually use vpmovmskb or an equivalent intrinsic if they want to collect the output of a comparison operator into a packed format.

@Majora320 Majora320 added the bug Observed behavior contradicts documented or intended behavior label Apr 11, 2025
@alexrp alexrp added the translate-c C to Zig source translation feature (@cImport) label Apr 11, 2025
@Majora320
Copy link
Author

Majora320 commented Apr 13, 2025

Would there be any interest in a PR that changes Zig's SIMD semantics to match that of C and other languages by modifying the result type of comparison operators from a packed bit vector to the source vector type? I might be interested in implementing this, but it would be a fair bit of work, especially the changes to the assembly generation for the direct compiler backends - some of that might be outside of my expertise. I'd like to gauge first whether this would be a welcome change or whether there's a good rationale for the current behavior or a desire for things not to change (as this change would break existing SIMD code).

@rohlem
Copy link
Contributor

rohlem commented Apr 13, 2025

I think there would be value in both - the boolean type of status-quo clarifies that logically we're dealing with true/false values
better than an integer type would, which helps readability of the code.

Maybe we could introduce the concept of different backing types (/layouts) for vector types?
That would allow expressing a vector of N bools with element-width 32bits (the immediate hardware result),
but also with element-width of 1 bit (= packed as in status-quo).

(It would also be possible to integrate #23327 like this:
merge arrays and vectors into one archetype, but give all the configuration options to model/configure the distinct status-quo types.)

@Majora320
Copy link
Author

The concept of different backing types is interesting, but I'm not sure if the added semantic clarity would be worth the increased complexity in the type system - differently "tagged" arrays would likely have to end up being different and incompatible types, as vector types already are, making the abstraction somewhat leaky by design/necessity. IMO Zig in general has a design philosophy of removing/reducing abstraction where possible, and sticking to that is probably a good idea in most cases.

My main argument for this change would be that in the case where the developer is reaching for explicit SIMD, they already likely want as much direct control over the hardware as possible and are likely willing to sacrifice some semantic clarity in exchange for that control (and performance).

@aqrit
Copy link

aqrit commented Apr 13, 2025

AVX-512 (and others?) use mask registers. Which (afaik) closely resemble the current semantics.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Observed behavior contradicts documented or intended behavior translate-c C to Zig source translation feature (@cImport)
Projects
None yet
Development

No branches or pull requests

4 participants