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

[Richard Biener <richard dot guenther at gmail dot com>]

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).

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

>  {
>    struct expand_operand ops[2];
>    tree type, lhs, rhs;
> @@ -127,7 +127,7 @@ expand_load_lanes_optab_fn (gcall *stmt, convert_optab optab)
>  /* Expand STORE_LANES call STMT using optab OPTAB.  */
>
>  static void
> -expand_store_lanes_optab_fn (gcall *stmt, convert_optab optab)
> +expand_store_lanes_optab_fn (internal_fn, gcall *stmt, convert_optab optab)
>  {
>    struct expand_operand ops[2];
>    tree type, lhs, rhs;
> @@ -149,7 +149,7 @@ expand_store_lanes_optab_fn (gcall *stmt, convert_optab optab)
>  }
>
>  static void
> -expand_ANNOTATE (gcall *)
> +expand_ANNOTATE (internal_fn, gcall *)
>  {
>    gcc_unreachable ();
>  }
> @@ -157,7 +157,7 @@ expand_ANNOTATE (gcall *)
>  /* This should get expanded in adjust_simduid_builtins.  */
>
>  static void
> -expand_GOMP_SIMD_LANE (gcall *)
> +expand_GOMP_SIMD_LANE (internal_fn, gcall *)
>  {
>    gcc_unreachable ();
>  }
> @@ -165,7 +165,7 @@ expand_GOMP_SIMD_LANE (gcall *)
>  /* This should get expanded in adjust_simduid_builtins.  */
>
>  static void
> -expand_GOMP_SIMD_VF (gcall *)
> +expand_GOMP_SIMD_VF (internal_fn, gcall *)
>  {
>    gcc_unreachable ();
>  }
> @@ -173,7 +173,7 @@ expand_GOMP_SIMD_VF (gcall *)
>  /* This should get expanded in adjust_simduid_builtins.  */
>
>  static void
> -expand_GOMP_SIMD_LAST_LANE (gcall *)
> +expand_GOMP_SIMD_LAST_LANE (internal_fn, gcall *)
>  {
>    gcc_unreachable ();
>  }
> @@ -181,7 +181,7 @@ expand_GOMP_SIMD_LAST_LANE (gcall *)
>  /* This should get expanded in adjust_simduid_builtins.  */
>
>  static void
> -expand_GOMP_SIMD_ORDERED_START (gcall *)
> +expand_GOMP_SIMD_ORDERED_START (internal_fn, gcall *)
>  {
>    gcc_unreachable ();
>  }
> @@ -189,7 +189,7 @@ expand_GOMP_SIMD_ORDERED_START (gcall *)
>  /* This should get expanded in adjust_simduid_builtins.  */
>
>  static void
> -expand_GOMP_SIMD_ORDERED_END (gcall *)
> +expand_GOMP_SIMD_ORDERED_END (internal_fn, gcall *)
>  {
>    gcc_unreachable ();
>  }
> @@ -197,7 +197,7 @@ expand_GOMP_SIMD_ORDERED_END (gcall *)
>  /* This should get expanded in the sanopt pass.  */
>
>  static void
> -expand_UBSAN_NULL (gcall *)
> +expand_UBSAN_NULL (internal_fn, gcall *)
>  {
>    gcc_unreachable ();
>  }
> @@ -205,7 +205,7 @@ expand_UBSAN_NULL (gcall *)
>  /* This should get expanded in the sanopt pass.  */
>
>  static void
> -expand_UBSAN_BOUNDS (gcall *)
> +expand_UBSAN_BOUNDS (internal_fn, gcall *)
>  {
>    gcc_unreachable ();
>  }
> @@ -213,7 +213,7 @@ expand_UBSAN_BOUNDS (gcall *)
>  /* This should get expanded in the sanopt pass.  */
>
>  static void
> -expand_UBSAN_VPTR (gcall *)
> +expand_UBSAN_VPTR (internal_fn, gcall *)
>  {
>    gcc_unreachable ();
>  }
> @@ -221,7 +221,7 @@ expand_UBSAN_VPTR (gcall *)
>  /* This should get expanded in the sanopt pass.  */
>
>  static void
> -expand_UBSAN_OBJECT_SIZE (gcall *)
> +expand_UBSAN_OBJECT_SIZE (internal_fn, gcall *)
>  {
>    gcc_unreachable ();
>  }
> @@ -229,7 +229,7 @@ expand_UBSAN_OBJECT_SIZE (gcall *)
>  /* This should get expanded in the sanopt pass.  */
>
>  static void
> -expand_ASAN_CHECK (gcall *)
> +expand_ASAN_CHECK (internal_fn, gcall *)
>  {
>    gcc_unreachable ();
>  }
> @@ -237,7 +237,7 @@ expand_ASAN_CHECK (gcall *)
>  /* This should get expanded in the tsan pass.  */
>
>  static void
> -expand_TSAN_FUNC_EXIT (gcall *)
> +expand_TSAN_FUNC_EXIT (internal_fn, gcall *)
>  {
>    gcc_unreachable ();
>  }
> @@ -1639,7 +1639,7 @@ expand_mul_overflow (location_t loc, tree lhs, tree arg0, tree arg1,
>  /* Expand UBSAN_CHECK_ADD call STMT.  */
>
>  static void
> -expand_UBSAN_CHECK_ADD (gcall *stmt)
> +expand_UBSAN_CHECK_ADD (internal_fn, gcall *stmt)
>  {
>    location_t loc = gimple_location (stmt);
>    tree lhs = gimple_call_lhs (stmt);
> @@ -1652,7 +1652,7 @@ expand_UBSAN_CHECK_ADD (gcall *stmt)
>  /* Expand UBSAN_CHECK_SUB call STMT.  */
>
>  static void
> -expand_UBSAN_CHECK_SUB (gcall *stmt)
> +expand_UBSAN_CHECK_SUB (internal_fn, gcall *stmt)
>  {
>    location_t loc = gimple_location (stmt);
>    tree lhs = gimple_call_lhs (stmt);
> @@ -1668,7 +1668,7 @@ expand_UBSAN_CHECK_SUB (gcall *stmt)
>  /* Expand UBSAN_CHECK_MUL call STMT.  */
>
>  static void
> -expand_UBSAN_CHECK_MUL (gcall *stmt)
> +expand_UBSAN_CHECK_MUL (internal_fn, gcall *stmt)
>  {
>    location_t loc = gimple_location (stmt);
>    tree lhs = gimple_call_lhs (stmt);
> @@ -1853,7 +1853,7 @@ expand_arith_overflow (enum tree_code code, gimple *stmt)
>  /* Expand ADD_OVERFLOW STMT.  */
>
>  static void
> -expand_ADD_OVERFLOW (gcall *stmt)
> +expand_ADD_OVERFLOW (internal_fn, gcall *stmt)
>  {
>    expand_arith_overflow (PLUS_EXPR, stmt);
>  }
> @@ -1861,7 +1861,7 @@ expand_ADD_OVERFLOW (gcall *stmt)
>  /* Expand SUB_OVERFLOW STMT.  */
>
>  static void
> -expand_SUB_OVERFLOW (gcall *stmt)
> +expand_SUB_OVERFLOW (internal_fn, gcall *stmt)
>  {
>    expand_arith_overflow (MINUS_EXPR, stmt);
>  }
> @@ -1869,7 +1869,7 @@ expand_SUB_OVERFLOW (gcall *stmt)
>  /* Expand MUL_OVERFLOW STMT.  */
>
>  static void
> -expand_MUL_OVERFLOW (gcall *stmt)
> +expand_MUL_OVERFLOW (internal_fn, gcall *stmt)
>  {
>    expand_arith_overflow (MULT_EXPR, stmt);
>  }
> @@ -1877,7 +1877,7 @@ expand_MUL_OVERFLOW (gcall *stmt)
>  /* This should get folded in tree-vectorizer.c.  */
>
>  static void
> -expand_LOOP_VECTORIZED (gcall *)
> +expand_LOOP_VECTORIZED (internal_fn, gcall *)
>  {
>    gcc_unreachable ();
>  }
> @@ -1885,7 +1885,7 @@ expand_LOOP_VECTORIZED (gcall *)
>  /* Expand MASK_LOAD call STMT using optab OPTAB.  */
>
>  static void
> -expand_mask_load_optab_fn (gcall *stmt, direct_optab optab)
> +expand_mask_load_optab_fn (internal_fn, gcall *stmt, direct_optab optab)
>  {
>    struct expand_operand ops[3];
>    tree type, lhs, rhs, maskt;
> @@ -1912,7 +1912,7 @@ expand_mask_load_optab_fn (gcall *stmt, direct_optab optab)
>  /* Expand MASK_STORE call STMT using optab OPTAB.  */
>
>  static void
> -expand_mask_store_optab_fn (gcall *stmt, direct_optab optab)
> +expand_mask_store_optab_fn (internal_fn, gcall *stmt, direct_optab optab)
>  {
>    struct expand_operand ops[3];
>    tree type, lhs, rhs, maskt;
> @@ -1935,12 +1935,12 @@ expand_mask_store_optab_fn (gcall *stmt, direct_optab optab)
>  }
>
>  static void
> -expand_ABNORMAL_DISPATCHER (gcall *)
> +expand_ABNORMAL_DISPATCHER (internal_fn, gcall *)
>  {
>  }
>
>  static void
> -expand_BUILTIN_EXPECT (gcall *stmt)
> +expand_BUILTIN_EXPECT (internal_fn, gcall *stmt)
>  {
>    /* When guessing was done, the hints should be already stripped away.  */
>    gcc_assert (!flag_guess_branch_prob || optimize == 0 || seen_error ());
> @@ -1960,7 +1960,7 @@ expand_BUILTIN_EXPECT (gcall *stmt)
>     should never be called.  */
>
>  static void
> -expand_VA_ARG (gcall *stmt ATTRIBUTE_UNUSED)
> +expand_VA_ARG (internal_fn, gcall *)
>  {
>    gcc_unreachable ();
>  }
> @@ -1968,7 +1968,7 @@ expand_VA_ARG (gcall *stmt ATTRIBUTE_UNUSED)
>  /* Expand the IFN_UNIQUE function according to its first argument.  */
>
>  static void
> -expand_UNIQUE (gcall *stmt)
> +expand_UNIQUE (internal_fn, gcall *stmt)
>  {
>    rtx pattern = NULL_RTX;
>    enum ifn_unique_kind kind
> @@ -2014,7 +2014,7 @@ expand_UNIQUE (gcall *stmt)
>  /* The size of an OpenACC compute dimension.  */
>
>  static void
> -expand_GOACC_DIM_SIZE (gcall *stmt)
> +expand_GOACC_DIM_SIZE (internal_fn, gcall *stmt)
>  {
>    tree lhs = gimple_call_lhs (stmt);
>
> @@ -2035,7 +2035,7 @@ expand_GOACC_DIM_SIZE (gcall *stmt)
>  /* The position of an OpenACC execution engine along one compute axis.  */
>
>  static void
> -expand_GOACC_DIM_POS (gcall *stmt)
> +expand_GOACC_DIM_POS (internal_fn, gcall *stmt)
>  {
>    tree lhs = gimple_call_lhs (stmt);
>
> @@ -2056,7 +2056,7 @@ expand_GOACC_DIM_POS (gcall *stmt)
>  /* This is expanded by oacc_device_lower pass.  */
>
>  static void
> -expand_GOACC_LOOP (gcall *stmt ATTRIBUTE_UNUSED)
> +expand_GOACC_LOOP (internal_fn, gcall *)
>  {
>    gcc_unreachable ();
>  }
> @@ -2064,20 +2064,20 @@ expand_GOACC_LOOP (gcall *stmt ATTRIBUTE_UNUSED)
>  /* This is expanded by oacc_device_lower pass.  */
>
>  static void
> -expand_GOACC_REDUCTION (gcall *stmt ATTRIBUTE_UNUSED)
> +expand_GOACC_REDUCTION (internal_fn, gcall *)
>  {
>    gcc_unreachable ();
>  }
>
> -/* 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));
>
> @@ -2115,11 +2115,11 @@ expand_direct_optab_fn (gcall *stmt, direct_optab optab, unsigned int nargs)
>
>  /* Expanders for optabs that can use expand_direct_optab_fn.  */
>
> -#define expand_unary_optab_fn(STMT, OPTAB) \
> -  expand_direct_optab_fn (STMT, OPTAB, 1)
> +#define expand_unary_optab_fn(FN, STMT, OPTAB) \
> +  expand_direct_optab_fn (FN, STMT, OPTAB, 1)
>
> -#define expand_binary_optab_fn(STMT, OPTAB) \
> -  expand_direct_optab_fn (STMT, OPTAB, 2)
> +#define expand_binary_optab_fn(FN, STMT, OPTAB) \
> +  expand_direct_optab_fn (FN, STMT, OPTAB, 2)
>
>  /* RETURN_TYPE and ARGS are a return type and argument list that are
>     in principle compatible with FN (which satisfies direct_internal_fn_p).
> @@ -2215,9 +2215,9 @@ direct_internal_fn_supported_p (internal_fn fn, tree type)
>
>  #define DEF_INTERNAL_OPTAB_FN(CODE, FLAGS, OPTAB, TYPE) \
>    static void                                          \
> -  expand_##CODE (gcall *stmt)                          \
> +  expand_##CODE (internal_fn fn, gcall *stmt)          \
>    {                                                    \
> -    expand_##TYPE##_optab_fn (stmt, OPTAB##_optab);    \
> +    expand_##TYPE##_optab_fn (fn, stmt, OPTAB##_optab);        \
>    }
>  #include "internal-fn.def"
>
> @@ -2227,16 +2227,24 @@ direct_internal_fn_supported_p (internal_fn fn, tree type)
>         expand_<NAME> (gcall *stmt)
>
>     where STMT is the statement that performs the call. */
> -static void (*const internal_fn_expanders[]) (gcall *) = {
> +static void (*const internal_fn_expanders[]) (internal_fn, gcall *) = {
>  #define DEF_INTERNAL_FN(CODE, FLAGS, FNSPEC) expand_##CODE,
>  #include "internal-fn.def"
>    0
>  };
>
> +/* Expand STMT as though it were a call to internal function FN.  */
> +
> +void
> +expand_internal_call (internal_fn fn, gcall *stmt)
> +{
> +  internal_fn_expanders[fn] (fn, stmt);

That is, just don't pass 'fn' here?  Or do we ever expect
to have the same internal_fn_expander for different fn?

Otherwise looks ok to me.

Richard.

> +}
> +
>  /* Expand STMT, which is a call to internal function FN.  */
>
>  void
>  expand_internal_call (gcall *stmt)
>  {
> -  internal_fn_expanders[(int) gimple_call_internal_fn (stmt)] (stmt);
> +  expand_internal_call (gimple_call_internal_fn (stmt), stmt);
>  }
> diff --git a/gcc/internal-fn.h b/gcc/internal-fn.h
> index 31e895e..5ee43b8 100644
> --- a/gcc/internal-fn.h
> +++ b/gcc/internal-fn.h
> @@ -162,5 +162,6 @@ extern bool direct_internal_fn_supported_p (internal_fn, tree_pair);
>  extern bool direct_internal_fn_supported_p (internal_fn, tree);
>
>  extern void expand_internal_call (gcall *);
> +extern void expand_internal_call (internal_fn, gcall *);
>
>  #endif
>