This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] A new meta intrinsic header file for current and future x86 instrinsics.


On Wed, Nov 19, 2008 at 9:30 PM, rajagopal, dwarak
<dwarak.rajagopal@amd.com> wrote:
>>
>> If there are no further comments or objections from AMD people in the
>> next 48 hours, and since this header file is intended for _Intel_
>> intrinsics, it is OK for mainline.
>>
>> BTW: I guess there will be a header file (ammintrin.h ?) introduced to
>> include AMD defined intrinsics, so gcc can still add x86intrin.h that
>> will include both immintrin.h and eventual ammintrin.h. Hopefully,
> these
>> two intrinsics won't step onto each other toes.
>>
>> I hope this eventual solution is acceptable to both contributors.

> This proposal is not acceptable to AMD for the following reasons:
>
> - GCC is an open source compiler that is vendor agnostic. We do not want
> to see vendor specific intrinsic files, as the ISAs are architecture
> specific primarily. This is how it has been till now (eg: emmintrin.h,
> xmmintrin.h, etc). Also there are many examples when AMD adopted SSE
> instructions after Intel created them, and Intel adopted AMD
> instructions after AMD came up with them (latest example: popcnt).
>
> - We recommend that there is only 1 intrinsic file used for all x86
> intrinsics. This seemed to have been agreed upon
> (http://gcc.gnu.org/ml/gcc/2008-04/msg00081.html). We prefer that this
> file be neutral and vendor agnostic, which immintrin.h is not.

The majority of the patch is a rename from gmmintrin.h to avxintrin.h.
This part is not problematic at all and since 4.4 is not yet released
is OK to commit.

Let me explain why I think that introducing immintrin.h as the step in
the right direction for the "master plan" in a little more depth:

immintrin.h collects all subheaders that are relevant for _intel_
processors. It is just a header of headers, conditionally included on
various __SSEx__ defines. Situation up to now was, that we had to
include the latest Intel defined intrinsic, i.e. "gmmintrin.h" to get
Intel's intrinsics (and this file included all earlier includes down
to mmintrin.h) _AND_  latest AMD defined intrinsics, i.e.
"bmmintrin.h" that includes all intrinsics relevant to AMD processors
(but for some reason not mm3dnow.h).

When new intrinsics file was introduced, we have to fix all users to
include this new file. See for example log history for
test-all-intrinsics tests gcc.target/i386/sse-{12,13,14}.h. This is
simply not acceptable.

However, there is a need for cross-compiler compatibility between
various compilers, not only icc, FWIW. So, the proposal, to some
extent harmonized between "other compilers" is to introduce two
headers:

-immintrin.h for Intel's intrinsics
-<whatever>mmintrin.h for AMD's intrinsics, where I propose that
<whatever> is "a" for some consistency.

These two intrinsics will just conditionally switch on or off various
includes, please look at proposed immintrin.h. I think that AMD
defined will also include various SSEx headers (they are all protected
against double inclusion), hopefully also mm3dnow.h.

Finally, a top-level file will be introduced, that will include
immintrin.h and <a>mmintrin.h. The name of this intrinsic is not yet
fixed, but it has to be the same as for other compilers. IMO,
"x86intrin.h" is a good name, I hope it will stay. So, users have the
flexibility to include x86intrin.h, immintrin.h or <a>mmintrin.h,
depending on their needs.

By including "x86intrin.h", subheaders will be conditionally included
by various -msseX -m<whatever> flags. This is the reason that a lot of
work has been done on the infrastructure for "this -msseX option
enables various other -msseX and disables some other -msseX".

Now, the concrete example is gcc.target/i386/sse-12.c testcase. As you
can see, it now includes <bmmintrin.h> for AMD intrinsics and
<smmintrin.h> for Intel intrinsics. (Also it includes mm3dnow.h -
forgotten by AMD?). With a new scheme, it will just include
<x86intrin.h>, and everything will be controlled by various -msseX
options. Try to compile this file without -msse5 for example, you will
see where the problem lies.

So I propose that AMD introduces its own intrinsic file, following
immintrin.h example. There *will be* a vendor neutral x86intrin.h (or
whatever the name will be agreed to), and AMDs file will be included
through this vendor neutral header just in the same way as Intels
file.

Uros.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]