Re: [PATCH 1/8] __builtin_dynamic_object_size: Recognize builtin name

[Siddhesh Poyarekar <siddhesh@gotplt.org>]

On 10/12/21 19:12, Jakub Jelinek wrote:
> On Fri, Oct 08, 2021 at 03:44:25AM +0530, Siddhesh Poyarekar wrote:
>> --- a/gcc/builtins.c
>> +++ b/gcc/builtins.c
>> @@ -180,6 +180,7 @@ static rtx expand_builtin_memory_chk (tree, rtx, machine_mode,
>>   static void maybe_emit_chk_warning (tree, enum built_in_function);
>>   static void maybe_emit_sprintf_chk_warning (tree, enum built_in_function);
>>   static tree fold_builtin_object_size (tree, tree);
>> +static tree fold_builtin_dyn_object_size (tree, tree);
>>   
>>   unsigned HOST_WIDE_INT target_newline;
>>   unsigned HOST_WIDE_INT target_percent;
>> @@ -7910,6 +7911,7 @@ expand_builtin (tree exp, rtx target, rtx subtarget, machine_mode mode,
>>         return const0_rtx;
>>   
>>       case BUILT_IN_OBJECT_SIZE:
>> +    case BUILT_IN_DYN_OBJECT_SIZE:
>>         return expand_builtin_object_size (exp);
> 
> I'd strongly prefer BUILT_IN_DYNAMIC_OBJECT_SIZE, we have even longer
> builtin enums and the abbreviation will only lead to confusion.
> 
>> +/* Fold a call to __builtin_dynamic_object_size with arguments PTR and OST,
>> +   if possible.  */
>> +
>> +static tree
>> +fold_builtin_dyn_object_size (tree ptr, tree ost)
> 
> Also please don't abbreviate.

Got it, will fix.

>> +{
>> +  int object_size_type;
>> +
>> +  if (!valid_object_size_args (ptr, ost, &object_size_type))
>> +    return NULL_TREE;
>> +
>> +  /* __builtin_dynamic_object_size doesn't evaluate side-effects in its
>> +     arguments; if there are any side-effects, it returns (size_t) -1 for types
>> +     0 and 1 and (size_t) 0 for types 2 and 3.  */
>> +  if (TREE_SIDE_EFFECTS (ptr))
>> +    return build_int_cst_type (size_type_node, object_size_type < 2 ? -1 : 0);
> 
> If we want to commit this patch separately, then the more natural stub
> implementation would be fold it into a __builtin_object_size call
> (or call fold_builtin_object_size and only if it returns NULL_TREE fold
> it into the builtin call).  But I assume we do not want to do that and
> want to commit the whole series at once, therefore even this is good enough.

Ideally, it would be great to have the whole series go in at once but if 
we're not able to build consensus soon enough, I'll post this one patch 
for inclusion.

Thanks,
Siddhesh