On 07/21/16 14:08, Richard Biener wrote:
> On Thu, 21 Jul 2016, Bernd Edlinger wrote:
>
>> On 07/21/16 13:35, Richard Biener wrote:
>>> On Thu, 21 Jul 2016, Bernd Edlinger wrote:
>>>
>>>> On 07/21/16 13:25, Bernd Schmidt wrote:
>>>>>
>>>>>
>>>>> On 07/21/2016 01:16 PM, Jakub Jelinek wrote:
>>>>>> On Thu, Jul 21, 2016 at 11:04:48AM +0000, Bernd Edlinger wrote:
>>>>>>>    bool
>>>>>>> +gimple_alloca_call_p (const gimple *stmt)
>>>>>>> +{
>>>>>>> +  tree fndecl;
>>>>>>> +
>>>>>>> +  if (!is_gimple_call (stmt))
>>>>>>> +    return false;
>>>>>>> +
>>>>>>> +  fndecl = gimple_call_fndecl (stmt);
>>>>>>> +  if (fndecl && DECL_BUILT_IN_CLASS (fndecl) == BUILT_IN_NORMAL)
>>>>>>> +    switch (DECL_FUNCTION_CODE (fndecl))
>>>>>>> +      {
>>>>>>> +      case BUILT_IN_ALLOCA:
>>>>>>> +      case BUILT_IN_ALLOCA_WITH_ALIGN:
>>>>>>> +        return true;
>>>>>>> +      }
>>>>>>
>>>>>> This should have failed bootstrap because of -Wswitch-enum.
>>>>>> You need
>>>>>>       default:
>>>>>>         break;
>>>>>> in.
>>>>>>
>>>>>>> +    switch (DECL_FUNCTION_CODE (fndecl))
>>>>>>> +      {
>>>>>>> +      case BUILT_IN_ALLOCA:
>>>>>>> +      case BUILT_IN_ALLOCA_WITH_ALIGN:
>>>>>>> +        return true;
>>>>>>
>>>>>> Likewise here.
>>>>>>
>>>>> Or write it in the more natural way as an if.
>>>>>
>>>>
>>>> I'm open for that suggestion.
>>>>
>>>> Then I should probably also rewrite the switch statement
>>>> in special_function_p as an if.
>>>
>>> I think a switch is a good fit though I don't mind an if as we probably
>>> know we'll never get more than two alloca builtins (heh, you never know).
>>>
>>
>> Thanks, style-nits are always welcome for me.  I also do care about
>> that a lot.
>>
>> I will keep the switch at the moment, and continue the boot-strap
>> with the approved version.
>>
>> BTW: in the function below "is_tm_builtin" there is a single switch
>> in a block statement, we usually don't do that redundant braces...
>>
>>
>> Richard, do you still have objections against the builtin_setjmp patch
>> from https://gcc.gnu.org/ml/gcc-patches/2016-07/msg01225.html ?
>
> No, I think it is ok, thus, approved as well.  Good to see that
> special_function_p cleaned up ;)
>

Yeah that was about time!  But that function is still like a mine field.
And it looks like it has been waiting there for me, all these years ;)

Jeff has found a reference to "savectx" in Solaris10, so it is probably
not yet completely dead as I thought.

If we need to keep the handling of savectx in special_function_p, that
would mean there ought to be a new built-in function for savectx as
well IMO.

With that hint I have googled up a header file /usr/include/sys/proc.h
from 2002 where the function signature can be seen:

extern  void    savectx(kthread_t *);

But that is in a #ifdef _KERNEL block, which means it is a kernel
function.

All that is only necessary, because we try to fix broken header files :(
I mean they should simply add a __attribute__((returns_twice)) here.

So I think that means that it is not possible to fix a kernel-header
file with a fixinclude rule.  I have heard that gcc can be built
on solaris, but is gcc also used as a tool to generate the solaris
kernel?

Is it really justified to have a built-in function for that?
I mean, that would be there even for windows and linux where that
function name is not reserved?


Thanks
Bernd.

Reply via email to