Short-cut generation of simple built-in functions

[Richard Biener]

On Tue, Nov 10, 2015 at 10:24 PM, Richard Sandiford
<richard.sandiford@arm.com> wrote:
> 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)
>
> ?

There was talk to make calls use volatile to prevent CSE and friends.

Using gimple_has_side_effects is just the better check.

> 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?

Not sure if the tests should be in-sync.

I'm also not sure what you really want to check with

>>> +  /* 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 know you want to catch errno setting here but shouldn't that use a
different kind of check and only done for a specific subset of
functions?  For example do you want to replace sqrt() with
an IFN in case it has a VUSE (it will have that with -frounding-math)?
In this case you'll lose the implicit dependence on fesetround and
friends.  This implicit dependence is not checked by gimple_has_side_effects
either btw.

>>> +      && (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.

Ah, ok.

Thanks,
Richard.

> Thanks,
> Richard
>