This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH v3][C][ADA] use function descriptors instead of trampolines in C


Is there a change that we can move forward with this?

I think this is a very useful feature and might be especially
important if GCC is going to activate -Wtrampoline  with 
-Wall on some architectures.

Best,
Martin


Am Sonntag, den 04.11.2018, 21:48 +0100 schrieb Martin Uecker:
> Hi Joseph,
> 
> here is a new version of this patch which adds a warning
> for targets which do not support -fno-trampolines  and
> only runs the test case on architectures where this is
> supported. It seems that documentation for this general
> feature has improved in the meantime so I only mention
> C as supported.
> 
> 
> Best,
> Martin
> 
> diff --git a/gcc/ChangeLog b/gcc/ChangeLog
> index 5cf291da2d5..e75500c647a 100644
> --- a/gcc/ChangeLog
> +++ b/gcc/ChangeLog
> @@ -1,3 +1,13 @@
> +2018-11-03  Martin Uecker  <martin.uecker@med.uni-goettingen.de>
> +
> +	* common.opt (flag_trampolines): Change default.
> +	* calls.c (prepare_call_address): Remove check for
> +	flag_trampolines.  Decision is now made in FEs.
> +	* tree-nested.c (convert_tramp_reference_op): Likewise.
> +	* toplev.c (process_options): Add warning for -fno-
> trampolines on
> +	unsupported targets.
> +	* doc/invoke.texi (-fno-trampolines): Document support for
> C.
> +
>  2018-11-02  Aaron Sawdey  <acsawdey@linux.ibm.com>
>  
>  	* config/rs6000/rs6000-string.c
> (expand_strncmp_gpr_sequence): Pay
> diff --git a/gcc/ada/ChangeLog b/gcc/ada/ChangeLog
> index 73666129f55..a7462edfc71 100644
> --- a/gcc/ada/ChangeLog
> +++ b/gcc/ada/ChangeLog
> @@ -1,3 +1,8 @@
> +2018-11-03  Martin Uecker  <martin.uecker@med.uni-goettingen.de>
> +
> +	* gcc-interface/trans.c (Attribute_to_gnu): Add check for
> +	flag_trampolines.
> +
>  2018-10-22  Eric Botcazou  <ebotcazou@adacore.com>
>  
>  	* gcc-interface/utils.c (unchecked_convert): Use local
> variables for
> diff --git a/gcc/ada/gcc-interface/trans.c b/gcc/ada/gcc-
> interface/trans.c
> index ce2d43f989e..b79f2373c63 100644
> --- a/gcc/ada/gcc-interface/trans.c
> +++ b/gcc/ada/gcc-interface/trans.c
> @@ -1753,7 +1753,8 @@ Attribute_to_gnu (Node_Id gnat_node, tree
> *gnu_result_type_p, int attribute)
>  	      if ((attribute == Attr_Access
>  		   || attribute == Attr_Unrestricted_Access)
>  		  && targetm.calls.custom_function_descriptors > 0
> -		  && Can_Use_Internal_Rep (Etype (gnat_node)))
> +		  && Can_Use_Internal_Rep (Etype (gnat_node))
> +                  && (flag_trampolines != 1))
>  		FUNC_ADDR_BY_DESCRIPTOR (gnu_expr) = 1;
>  
>  	      /* Otherwise, we need to check that we are not
> violating the
> @@ -4330,7 +4331,8 @@ Call_to_gnu (Node_Id gnat_node, tree
> *gnu_result_type_p, tree gnu_target,
>        /* If the access type doesn't require foreign-compatible
> representation,
>  	 be prepared for descriptors.  */
>        if (targetm.calls.custom_function_descriptors > 0
> -	  && Can_Use_Internal_Rep (Etype (Prefix (Name
> (gnat_node)))))
> +	  && Can_Use_Internal_Rep (Etype (Prefix (Name
> (gnat_node))))
> +          && (flag_trampolines != 1))
>  	by_descriptor = true;
>      }
>    else if (Nkind (Name (gnat_node)) == N_Attribute_Reference)
> diff --git a/gcc/c/ChangeLog b/gcc/c/ChangeLog
> index 708ef5d7da2..62823ccf5c7 100644
> --- a/gcc/c/ChangeLog
> +++ b/gcc/c/ChangeLog
> @@ -1,3 +1,10 @@
> +2018-11-03  Martin Uecker  <martin.uecker@med.uni-goettingen.de>
> +
> +	* c-objc-common.h: Define
> LANG_HOOKS_CUSTOM_FUNCTION_DESCRIPTORS.
> +	* c-typeck.c (function_to_pointer_conversion): If using
> descriptors
> +	instead of trampolines, amend function address with
> +	FUNC_ADDR_BY_DESCRIPTOR and calls with
> ALL_EXPR_BY_DESCRIPTOR.
> +
>  2018-10-29  David Malcolm  <dmalcolm@redhat.com>
>  
>  	* c-decl.c (implicit_decl_warning): Update "is there a
> suggestion"
> diff --git a/gcc/c/c-objc-common.h b/gcc/c/c-objc-common.h
> index 78e768c2366..ef039560eb9 100644
> --- a/gcc/c/c-objc-common.h
> +++ b/gcc/c/c-objc-common.h
> @@ -110,4 +110,7 @@ along with GCC; see the file COPYING3.  If not
> see
>  
>  #undef LANG_HOOKS_TREE_INLINING_VAR_MOD_TYPE_P
>  #define LANG_HOOKS_TREE_INLINING_VAR_MOD_TYPE_P c_vla_unspec_p
> +
> +#undef LANG_HOOKS_CUSTOM_FUNCTION_DESCRIPTORS
> +#define LANG_HOOKS_CUSTOM_FUNCTION_DESCRIPTORS true
>  #endif /* GCC_C_OBJC_COMMON */
> diff --git a/gcc/c/c-typeck.c b/gcc/c/c-typeck.c
> index 9d09b8d65fd..afae9de41e7 100644
> --- a/gcc/c/c-typeck.c
> +++ b/gcc/c/c-typeck.c
> @@ -1912,7 +1912,13 @@ function_to_pointer_conversion (location_t
> loc, tree exp)
>    if (TREE_NO_WARNING (orig_exp))
>      TREE_NO_WARNING (exp) = 1;
>  
> -  return build_unary_op (loc, ADDR_EXPR, exp, false);
> +  tree r = build_unary_op (loc, ADDR_EXPR, exp, false);
> +
> +  if ((TREE_CODE(r) == ADDR_EXPR)
> +      && (flag_trampolines == 0))
> +     FUNC_ADDR_BY_DESCRIPTOR (r) = 1;
> +
> +  return r;
>  }
>  
>  /* Mark EXP as read, not just set, for set but not used -Wunused
> @@ -3134,6 +3140,11 @@ build_function_call_vec (location_t loc,
> vec<location_t> arg_loc,
>    else
>      result = build_call_array_loc (loc, TREE_TYPE (fntype),
>  				   function, nargs, argarray);
> +
> +  if ((TREE_CODE (result) == CALL_EXPR)
> +      && (flag_trampolines == 0))
> +    CALL_EXPR_BY_DESCRIPTOR (result) = 1;
> +
>    /* If -Wnonnull warning has been diagnosed, avoid diagnosing it
> again
>       later.  */
>    if (warned_p && TREE_CODE (result) == CALL_EXPR)
> diff --git a/gcc/calls.c b/gcc/calls.c
> index 8978d3b42fd..95ab7d8405b 100644
> --- a/gcc/calls.c
> +++ b/gcc/calls.c
> @@ -230,7 +230,7 @@ prepare_call_address (tree fndecl_or_type, rtx
> funexp, rtx static_chain_value,
>      {
>        /* If it's an indirect call by descriptor, generate code to
> perform
>  	 runtime identification of the pointer and load the
> descriptor.  */
> -      if ((flags & ECF_BY_DESCRIPTOR) && !flag_trampolines)
> +      if (flags & ECF_BY_DESCRIPTOR)
>  	{
>  	  const int bit_val =
> targetm.calls.custom_function_descriptors;
>  	  rtx call_lab = gen_label_rtx ();
> diff --git a/gcc/common.opt b/gcc/common.opt
> index 2971dc21b1f..8457c93edab 100644
> --- a/gcc/common.opt
> +++ b/gcc/common.opt
> @@ -2487,7 +2487,7 @@ Common Report Var(flag_tracer) Optimization
>  Perform superblock formation via tail duplication.
>  
>  ftrampolines
> -Common Report Var(flag_trampolines) Init(0)
> +Common Report Var(flag_trampolines) Init(-1)
>  For targets that normally need trampolines for nested functions,
> always
>  generate them instead of using descriptors.
>  
> diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
> index e290128f535..ccf651c1354 100644
> --- a/gcc/doc/invoke.texi
> +++ b/gcc/doc/invoke.texi
> @@ -13570,7 +13570,8 @@ made executable in order for the program to
> work properly.
>  basis to let the compiler avoid generating them, if it computes that
> this
>  is safe, and replace them with descriptors.  Descriptors are made up
> of data
>  only, but the generated code must be prepared to deal with them.  As
> of this
> -writing, @option{-fno-trampolines} is enabled by default only for
> Ada.
> +writing, @option{-fno-trampolines} is supported only for C and Ada
> and
> +enabled by default only for Ada.
>  
>  Moreover, code compiled with @option{-ftrampolines} and code
> compiled with
>  @option{-fno-trampolines} are not binary compatible if nested
> functions are
> diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog
> index a4d50614537..9c4bce69bd9 100644
> --- a/gcc/testsuite/ChangeLog
> +++ b/gcc/testsuite/ChangeLog
> @@ -1,3 +1,9 @@
> +2018-11-03  Martin Uecker  <martin.uecker@med.uni-goettingen.de>
> +
> +	* gcc.dg/trampoline-2.c: New test.
> +	* lib/target-supports.exp 
> +	(check_effective_target_notrampolines): New.
> +
>  2018-11-02  Rainer Orth  <ro@CeBiTec.Uni-Bielefeld.DE>
>  
>  	* gcc.dg/compat/pr83487-1_y.c: Move dg-skip-if ...
> diff --git a/gcc/testsuite/gcc.dg/trampoline-2.c
> b/gcc/testsuite/gcc.dg/trampoline-2.c
> new file mode 100644
> index 00000000000..06c1cf4f647
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/trampoline-2.c
> @@ -0,0 +1,23 @@
> +/* test that nested function work without trampolines for -fno-
> trampolines */
> +/* Origin: Martin Uecker <martin.uecker@med.uni-goettingen.de> */
> +/* { dg-require-effective-target notrampolines } */
> +/* { dg-options "-std=gnu11 -O2 -Wtrampolines -fno-trampolines" } */
> +
> +static int p(void) { return +1; }
> +static int m(void) { return -1; } 
> +static int z(void) { return 0; }
> +
> +typedef int (*funptr_t)(void);
> +
> +static int A(int k, funptr_t a1, funptr_t a2, funptr_t a3, funptr_t
> a4, funptr_t a5)
> +{
> +	int B(void) { return A(--k, B, a1, a2, a3, a4); }
> +
> +	return (k <= 0) ? (a4() + a5()) : (B());
> +}
> +
> +int main(void)
> +{
> +	return (0 == A(5, p, m, m, p, z)) ? 0 : 1;
> +}
> +
> diff --git a/gcc/testsuite/lib/target-supports.exp
> b/gcc/testsuite/lib/target-supports.exp
> index fd74c04d092..a34e966b7c4 100644
> --- a/gcc/testsuite/lib/target-supports.exp
> +++ b/gcc/testsuite/lib/target-supports.exp
> @@ -916,6 +916,14 @@ proc check_effective_target_scheduling {} {
>      } "-fschedule-insns"]
>  }
>  
> +# Return 1 if it is possible to use function descriptors instead of
> trampolines, 0 otherwise.
> +
> +proc check_effective_target_notrampolines {} {
> +    return [check_no_compiler_messages notrampolines assembly {
> +        void foo (void) { }
> +    } "-fno-trampolines"]
> +}
> +
>  # Return 1 if trapping arithmetic is available, 0 otherwise.
>  
>  proc check_effective_target_trapping {} {
> diff --git a/gcc/toplev.c b/gcc/toplev.c
> index d7ea11abf53..33ded241a20 100644
> --- a/gcc/toplev.c
> +++ b/gcc/toplev.c
> @@ -1697,6 +1697,12 @@ process_options (void)
>        flag_prefetch_loop_arrays = 0;
>      }
>  
> +  if (flag_trampolines == 0 &&
> targetm.calls.custom_function_descriptors == -1)
> +   {
> +     warning_at (UNKNOWN_LOCATION, 0,
> +                 "-fno-trampolines not supported for this target");
> +   }
> +
>    /* This combination of options isn't handled for i386 targets and
> doesn't
>       make much sense anyway, so don't allow it.  */
>    if (flag_prefetch_loop_arrays > 0 && optimize_size)
> diff --git a/gcc/tree-nested.c b/gcc/tree-nested.c
> index 4579b4c5839..e95c9aea051 100644
> --- a/gcc/tree-nested.c
> +++ b/gcc/tree-nested.c
> @@ -2499,7 +2499,7 @@ convert_tramp_reference_op (tree *tp, int
> *walk_subtrees, void *data)
>  	continue;
>  
>        /* Decide whether to generate a descriptor or a trampoline. */
> -      descr = FUNC_ADDR_BY_DESCRIPTOR (t) && !flag_trampolines;
> +      descr = FUNC_ADDR_BY_DESCRIPTOR (t);
>  
>        if (descr)
>  	x = lookup_descr_for_decl (i, decl, INSERT);

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]