Re: Short-cut generation of simple built-in functions

[Richard Sandiford <richard dot sandiford at arm dot com>]

Richard Biener <richard.guenther@gmail.com> writes:
> On Sat, Nov 7, 2015 at 2:31 PM, Richard Sandiford
> <richard.sandiford@arm.com> wrote:
>> This patch short-circuits the builtins.c expansion code for a particular
>> gimple call if:
>>
>> - the function has an associated internal function
>> - the target implements that internal function
>> - the call has no side effects
>>
>> This allows a later patch to remove the builtins.c code, once calls with
>> side effects have been handled.
>>
>> Tested on x86_64-linux-gnu, aarch64-linux-gnu and arm-linux-gnueabi.
>> OK to install?
>>
>> Thanks,
>> Richard
>>
>>
>> gcc/
>>         * builtins.h (called_as_built_in): Declare.
>>         * builtins.c (called_as_built_in): Make external.
>>         * internal-fn.h (expand_internal_call): Define a variant that
>>         specifies the internal function explicitly.
>>         * internal-fn.c (expand_load_lanes_optab_fn)
>>         (expand_store_lanes_optab_fn, expand_ANNOTATE, expand_GOMP_SIMD_LANE)
>>         (expand_GOMP_SIMD_VF, expand_GOMP_SIMD_LAST_LANE)
>>         (expand_GOMP_SIMD_ORDERED_START, expand_GOMP_SIMD_ORDERED_END)
>>         (expand_UBSAN_NULL, expand_UBSAN_BOUNDS, expand_UBSAN_VPTR)
>>         (expand_UBSAN_OBJECT_SIZE, expand_ASAN_CHECK, expand_TSAN_FUNC_EXIT)
>>         (expand_UBSAN_CHECK_ADD, expand_UBSAN_CHECK_SUB)
>>         (expand_UBSAN_CHECK_MUL, expand_ADD_OVERFLOW, expand_SUB_OVERFLOW)
>>         (expand_MUL_OVERFLOW, expand_LOOP_VECTORIZED)
>>         (expand_mask_load_optab_fn, expand_mask_store_optab_fn)
>>         (expand_ABNORMAL_DISPATCHER, expand_BUILTIN_EXPECT, expand_VA_ARG)
>>         (expand_UNIQUE, expand_GOACC_DIM_SIZE, expand_GOACC_DIM_POS)
>>         (expand_GOACC_LOOP, expand_GOACC_REDUCTION, expand_direct_optab_fn)
>>         (expand_unary_optab_fn, expand_binary_optab_fn): Add an internal_fn
>>         argument.
>>         (internal_fn_expanders): Update prototype.
>>         (expand_internal_call): Define a variant that specifies the
>>         internal function explicitly. Use it to implement the previous
>>         interface.
>>         * cfgexpand.c (expand_call_stmt): Try to expand calls to built-in
>>         functions as calls to internal functions.
>>
>> diff --git a/gcc/builtins.c b/gcc/builtins.c
>> index f65011e..bbcc7dc3 100644
>> --- a/gcc/builtins.c
>> +++ b/gcc/builtins.c
>> @@ -222,7 +222,7 @@ is_builtin_fn (tree decl)
>>     of the optimization level.  This means whenever a function is invoked with
>>     its "internal" name, which normally contains the prefix "__builtin".  */
>>
>> -static bool
>> +bool
>>  called_as_built_in (tree node)
>>  {
>>    /* Note that we must use DECL_NAME, not DECL_ASSEMBLER_NAME_SET_P since
>> diff --git a/gcc/builtins.h b/gcc/builtins.h
>> index 917eb90..1d00068 100644
>> --- a/gcc/builtins.h
>> +++ b/gcc/builtins.h
>> @@ -50,6 +50,7 @@ extern struct target_builtins *this_target_builtins;
>>  extern bool force_folding_builtin_constant_p;
>>
>>  extern bool is_builtin_fn (tree);
>> +extern bool called_as_built_in (tree);
>>  extern bool get_object_alignment_1 (tree, unsigned int *,
>>                                     unsigned HOST_WIDE_INT *);
>>  extern unsigned int get_object_alignment (tree);
>> diff --git a/gcc/cfgexpand.c b/gcc/cfgexpand.c
>> index bfbc958..dc7d4f5 100644
>> --- a/gcc/cfgexpand.c
>> +++ b/gcc/cfgexpand.c
>> @@ -2551,10 +2551,25 @@ expand_call_stmt (gcall *stmt)
>>        return;
>>      }
>>
>> +  /* If this is a call to a built-in function and it has no effect other
>> +     than setting the lhs, try to implement it using an internal function
>> +     instead.  */
>> +  decl = gimple_call_fndecl (stmt);
>> +  if (gimple_call_lhs (stmt)
>> +      && !gimple_vdef (stmt)
>
> I think you want && ! gimple_has_side_effects (stmt)
> instead of checking !gimple_vdef (stmt).

OK, I can do that, but what would the difference be in practice for
these types of call?  I.e. are there cases for built-ins where:

  (A) gimple_vdef (stmt) && !gimple_side_effects (stmt)

or:

  (B) !gimple_vdef (stmt) && gimple_side_effects (stmt)

?

It just seems like this check should be the opposite of the one used
in the call-cdce patch (when deciding whether to optimise a call
with an lhs).  In order to keep them in sync I'd need to use
gimple_side_effects rather than gimple_vdef there too, but is
(B) a possibility there?

>> +      && (optimize || (decl && called_as_built_in (decl))))
>> +    {
>> +      internal_fn ifn = replacement_internal_fn (stmt);
>> +      if (ifn != IFN_LAST)
>> +       {
>> +         expand_internal_call (ifn, stmt);
>> +         return;
>> +       }
>> +    }
>> +
>>    exp = build_vl_exp (CALL_EXPR, gimple_call_num_args (stmt) + 3);
>>
>>    CALL_EXPR_FN (exp) = gimple_call_fn (stmt);
>> -  decl = gimple_call_fndecl (stmt);
>>    builtin_p = decl && DECL_BUILT_IN (decl);
>>
>>    /* If this is not a builtin function, the function type through which the
>> diff --git a/gcc/internal-fn.c b/gcc/internal-fn.c
>> index 9f9f9cf..c03c0fc 100644
>> --- a/gcc/internal-fn.c
>> +++ b/gcc/internal-fn.c
>> @@ -103,7 +103,7 @@ get_multi_vector_move (tree array_type,
> convert_optab optab)
>>  /* Expand LOAD_LANES call STMT using optab OPTAB.  */
>>
>>  static void
>> -expand_load_lanes_optab_fn (gcall *stmt, convert_optab optab)
>> +expand_load_lanes_optab_fn (internal_fn, gcall *stmt, convert_optab optab)
>
> I'm somewhat confused by all these unused internal_fn args.  I'm sure
> we can avoid them?

They're needed for:

>> -/* Expand call STMT using OPTAB, which has a single output operand and
>> -   NARGS input operands.  */
>> +/* Expand a call to FN using the operands in STMT.  FN has a single
>> +   output operand and NARGS input operands.  */
>>
>>  static void
>> -expand_direct_optab_fn (gcall *stmt, direct_optab optab, unsigned int nargs)
>> +expand_direct_optab_fn (internal_fn fn, gcall *stmt, direct_optab optab,
>> +                       unsigned int nargs)
>>  {
>>    expand_operand *ops = XALLOCAVEC (expand_operand, nargs + 1);
>>
>> -  internal_fn fn = gimple_call_internal_fn (stmt);
>>    tree_pair types = direct_internal_fn_types (fn, stmt);
>>    insn_code icode = direct_optab_handler (optab, TYPE_MODE (types.first));

where we previously assumed that the function was the same as the call.

Thanks,
Richard