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

[Sriraman Tallam <tmsriram at google dot com>]

Ping.

On Wed, Apr 17, 2013 at 7:13 PM, Sriraman Tallam <tmsriram@google.com> wrote:
> +HJ
>
> On Tue, Apr 16, 2013 at 1:54 PM, Sriraman Tallam <tmsriram@google.com> wrote:
>> 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