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 ?
Bernd.
> Richard.
>
>> Thanks
>> Bernd.
>>
>>>
>>> Bernd
>>>
>>
>>
>
