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: Enable Intel AES/CLMUL


On Fri, Apr 4, 2008 at 2:48 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Fri, Apr 04, 2008 at 08:30:40AM +0200, Uros Bizjak wrote:
>  > On Thu, Apr 3, 2008 at 11:51 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
>  >
>  > >  +  /* Enable SSE 4.2 if AES or CLMUL is enabled.  */
>  > >  +  if ((x86_aes || x86_clmul)
>  > >  +      && !(ix86_isa_flags_explicit & OPTION_MASK_ISA_SSE4_2))
>  > >  +    {
>  > >  +      ix86_isa_flags |= OPTION_MASK_ISA_SSE4_2_SET;
>  > >  +      ix86_isa_flags_explicit |= OPTION_MASK_ISA_SSE4_2_SET;
>  > >  +    }
>  > >  +
>  >
>  > Do we really want to enable all SSE builtins if only AES functionality
>  > is required? I think that -msse -maes is more appropriate, since -msse
>  > instructs the compiler to enable support for SSE registers and -maes
>  > enables special AES instructions that depend on SSE registers.
>  > Unfortunately, we don't have separate option to enable only SSE
>  > registers without SSE builtins, but IMO we can live with that. Similar
>  > situation is with -mclmul. At minimum, -msse -mclmul should be
>  > required, I see no reason to enable everything up to sse4_2 for this
>  > insn.
>
>  SSE doesn't support V2DI. You need at least SSE2.
>

OK.

>
>  >
>  > >  +/* We need definitions from the SSE4, SSSE3, SSE3, SSE2 and SSE header
>  > >  +   files.  */
>  > >  +#include <smmintrin.h>
>  > >  +
>  >
>  > Actually we only need __v2di and __m128i typedefs here. Perhaps we can
>  > copy these two definitions here, or we can define
>  >
>  > typedef long long __m128a __attribute__ ((vector_size (16), __may_alias));
>  >
>  > and rewrite these new intrinsics to use this type?
>
>  <wmmintrin.h> from icc 10.1 includes SSE4 intrinsic header file. If
>  <wmmintrin.h> from gcc doesn't support SSE4 intrinsics, it will be
>  incompatible with icc. I can image other companies may implement
>  AES/CLMUL. I think it should be handled similar to SSE4 intrinsics.
>  That is we put AES/CLMUL intrinsics in a common intrinsic header file
>  and 2 different options, one of them is -maes/-mclmul, will enale
>  them.  -maes/-mclmul will still imply SSE4.

This practically forces usage of -msse4.1 for -maes/-mclmul, since
these instructions are used only through intrinsics, defined in their
corresponding header file. wmmintrin.h includes smmintrin.h that will
error out by #ifndef __SSE4_1__.

I think that user should explicitly enable sse4.1 manually in the
command line together with -maes/-mclmul to access these new
instructions. Including wmmintrin.h will error out when neither
-maes/-mclmul is enabled, and smmintrin.h will error out with a
message that SSE4.1 instruction set is not enabled.

Does icc automatically enable all SSE intrinsics for -maes/-mclmul?

>  I can change
>
>
>  { OPTION_MASK_ISA_SSE4_2, CODE_FOR_aesimc, 0, IX86_BUILTIN_AESIMC128, UNKNOWN , 0 },
>
>  to
>
>  { OPTION_MASK_ISA_SSE2, CODE_FOR_aesimc, 0, IX86_BUILTIN_AESIMC128, UNKNOWN , 0 },
>

Since there is no other way to generate these insns than through
intrinsics (__builtin_X should not be used by user as this is not
considered stable interface), we can put OPTION_MASK_ISA_SSE4_1 here.
At least SSE4_1 should be active due to constraint forced by
wmmintrin.h/smmintrin.h

> +#if !defined (__AES__) && !defined (__CLMUL__)
> +# error "AES/CLMUL instruction set not enabled"
> +#else

"instruction set" or "instructions" in the error message?

Uros.


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