Page MenuHomePhabricator

[X86] Move all Intel defined intrinsic includes into immintrin.h
ClosedPublic

Authored by craig.topper on May 21 2018, 11:08 PM.

Details

Summary

This matches the Intel documentation which shows them available by importing immintrin.h. x86intrin.h also includes immintrin.h so anyone including x86intrin.h will still get them.

This is different than gcc, but I don't think we were a perfect match there already. I'm unclear what gcc's policy is about how they choose which to add things to.

Diff Detail

Event Timeline

craig.topper created this revision.May 21 2018, 11:08 PM

What does "imm" mean anyways?

First there was mmintrin.h which covered MMX instructions. Then xmmintrin.h came along to support SSE1 and implicitly included mmintrin.h. The emmintrin.h to support SSE2 and implicitly included xmmintrin.h. This repeated for each new version of SSE. With each header file including the previous header file. I think most of the later SSE headers start with the first letter of the code name of the CPU that added that SSE level including cancelled projects. So nmmintrin.h refers to Nehalem. tmmintrin.h came from the cancelled Tejas CPU. wmmintrin.h referes to Westmere. pmmintrin.h refers to Penryn. Eventually this was determined to not be very scalable to remember which header file contained what intrinsics and you have to change it with each generation to get the latest.. So immintrin.h was created to just include everything. I assume the 'i' here just stands for Intel. The 'mm' is just historic legacy due to the earlier file names. x86intrin.h was created by gcc to hold the intrinsics not specified by Intel. I think icc has an x86intrin.h that just includes immintrin.h.

rnk accepted this revision.May 22 2018, 10:22 AM

Eventually this was determined to not be very scalable to remember which header file contained what intrinsics and you have to change it with each generation to get the latest.. So immintrin.h was created to just include everything.

Speaking of things that aren't scalable... immintrin.h is like the new windows.h. :(

The reorganization looks good to me, though.

This revision is now accepted and ready to land.May 22 2018, 10:22 AM

I agree with the changes in x86intrin.h and immintrin.h. For the others, I question whether we ought to recommend inclusion of x86intrin.h or immintrin.h. The distinction as I understand it is that immintrin.h is used for Intel-specific intrinsics while x86intrin.h covers all intrinsics for x86-based architectures.

Leave the message still saying x86intrin.h. Change the error checks to look for either x86intrin.h or immintrin.h to have been included. Really only the immintrin.h check is necessary since that's the header that does the include, but I put both so the error message saying x86intrin.h would be less confusing.

craig.topper requested review of this revision.May 22 2018, 3:35 PM

Hi Craig, just one comment on the details. Everything else looks good.

lib/Headers/x86intrin.h
47

I see that you are removing this popcntintrin.h inclusion without adding it to immintrin.h. Is that because you are relying on the inclusion from smmintrin.h?

If so, won't that cause a problem on Windows with -mpopcnt for targets that don't include smmintrin.h?

Add back popcntintrin.h

DavidKreitzer accepted this revision.May 23 2018, 11:07 AM

Thanks, Craig. LGTM.

This revision is now accepted and ready to land.May 23 2018, 11:07 AM
This revision was automatically updated to reflect the committed changes.