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-specific C support library functions #68

Closed
nwf opened this issue May 15, 2020 · 4 comments
Closed

X86-specific C support library functions #68

nwf opened this issue May 15, 2020 · 4 comments

Comments

@nwf
Copy link

nwf commented May 15, 2020

I recognize that this is surely not a huge priority, but:

lib/int128/sail.c and lib/sail.c each make use of X86-specific built-ins and __attribute__((target ("bmi2"))). This means that the resulting C translations are necessarily post-Haswell X86 only, which is not great (though one assumes the performance gain from BMI2 for bit twiddling is considerable).

@Alasdair
Copy link
Collaborator

I think the only one I really use is bzhi_u64. I remember the performance boost was nice (but not earth shattering) on my Intel desktop in the Lab, but now I'm using an AMD ryzen on my home computer and I think the bmi instructions are actually slower than the shift and mask version:

bzhi_u64(bits,len) = bits & (UINT64_MAX >> (64 - len))

So it might be worth making it optional anyway

@MattWindsor91
Copy link

I just ran into this, or something similar, on trying to compile sail-riscv on an Apple M1 computer (ie, AArch64) - the inclusion of x86intrin.h in sail.c produced the following (rather strange) error:

In file included from sail.c:10:
In file included from /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/lib/clang/12.0.0/include/x86intrin.h:15:
In file included from /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/lib/clang/12.0.0/include/immintrin.h:14:
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/lib/clang/12.0.0/include/mmintrin.h:50:12: error: invalid conversion between vector type
      '__m64' (vector of 1 'long long' value) and integer type 'int' of different size
    return (__m64)__builtin_ia32_vec_init_v2si(__i, 0);
           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

and many other similar errors.

On my copy there are instances of _pdep_u64 and _bzhi_u64 in sail.c, both of which can presumably be stubbed out with less efficient C equivalents on things without x86 BMI2 - I'm not sure if there are any other x86 specific things in sail? Apologies if what I'm trying to do is explicitly unsupported/will cause issues elsewhere.

@Alasdair
Copy link
Collaborator

Alasdair commented Jan 4, 2021

I think it's probably best to just remove these intrinsics for portability now that arm based macs are something we will want to support

@Alasdair
Copy link
Collaborator

Alasdair commented Jan 8, 2021

Removed in 55a91e5

@Alasdair Alasdair closed this as completed Jan 8, 2021
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

No branches or pull requests

3 participants