Richard Biener <richard.guent...@gmail.com> writes:
> On Sat, Nov 7, 2015 at 2:31 PM, Richard Sandiford
> <richard.sandif...@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
