Re: GCC does not support *mmintrin.h with function specific opts

[Sriraman Tallam <tmsriram at google dot com>]

Hi,

I have attached an updated patch that  addresses all the comments raised.

On Fri, Apr 12, 2013 at 1:58 AM, Jakub Jelinek <jakub@redhat.com> wrote:
> On Thu, Apr 11, 2013 at 12:05:41PM -0700, Sriraman Tallam wrote:
>> I have attached a patch that fixes this. I have added an option
>> "-mgenerate-builtins" that will do two things.  It will define a macro
>> "__ALL_ISA__" which will expose the *intrin.h functions. It will also
>> expose all the target specific builtins.  -mgenerate-builtins will not
>> affect code generation.
>
> 1) this shouldn't be an option, either it can be made to work reliably,
>    then it should be done always, or it can't, then it shouldn't be done

Ok, it is on by default now.  There is a way to turn it off, with
-mno-generate-builtins.

> 2) have you verified that if you always generate all builtins, that the
>    builtins not supported by the ISA selected from the command line are
>    created with the right vector modes?

This issue does not arise.  When the target builtin is expanded, it is
checked if the ISA support is there, either via function specific
target opts or global target opts. If not, an error is raised. Test
case added for this, please see intrinsic_4.c in patch.

> 3) the *intrin.h headers in the case where the guarding macro isn't defined
>    should be surrounded by something like
>    #ifndef __FMA4__
>    #pragma GCC push options
>    #pragma GCC target("fma4")
>    #endif
>    ...
>    #ifndef __FMA4__
>    #pragma GCC pop options
>    #endif
>    so that everything that is in the headers is compiled with the ISA
>    in question

I do not think this should be done because it will break the inlining
ability of the header function and cause issues if the caller does not
specify the required ISA. The fact that the header functions are
marked extern __inline, with gnu_inline guarantees that a body will
not be generated and they will be inlined.  If the caller does not
have the required ISA, appropriate errors will be raised. Test cases
added, see intrinsics_1.c, intrinsics_2.c

> 4) what happens if you use the various vector types typedefed in the
>    *intrin.h headers in code that doesn't support those ISAs?  As TYPE_MODE
>    for VECTOR_TYPE is a function call, perhaps it will just be handled as
>    generic BLKmode vectors, which is desirable I think

I checked some tests here.  With -mno-sse for instance, vector types
are not permitted in function arguments and return values and gcc
raises a warning/error in each case.  With return values, gcc always
gives an error if a SSE register is required in a return value.  I
even fixed this message to not do it for functions marked as extern
inline, with "gnu_inline" keyword as a body for them will not be
generated.


> 5) what happens if you use a target builtin in a function not supporting
>    the corresponding ISA, do you get proper error explaining what you are
>    doing wrong?

Yes, please sse intrinsic_4.c test in patch.

> 6) what happens if you use some intrinsics in a function not supporting
>    the corresponding ISA?  Dunno if the inliner chooses not to inline it
>    and error out because it is always_inline, or what exactly will happen
>    then

Same deal here. The intrinsic function will, guaranteed, to be inlined
into the caller which will be a corresponding builtin call. That
builtin call will trigger an error if the ISA is not supported.

Thanks
Sri

>
> For all this you certainly need testcases.
>
>         Jakub

Attachment: mmintrin_patch.txt
Description: Text document