Hi Jeff,

thank you. I fixed all the minor issues, but see below.


Am Montag, den 03.12.2018, 14:56 -0700 schrieb Jeff Law:
> On 11/4/18 1:48 PM, Uecker, Martin wrote:
> > 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/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;
> 
> You've got an extraneous set of parenthesis around your flag_trampolines
> check.  Please remove them.
> 
> 
> >  
> >           /* 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))
> 
> Similarly here.
> 
> 
> > 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 */
> 
> I wonder if we even need the lang hook anymore.  ISTM that a front-end
> that wants to use the function descriptors can just set
> FUNC_ADDR_BY_DESCRIPTOR and we'd use the function descriptor, else we'll
> use the trampoline.  Thoughts?

The lang hook also affects the minimum alignment for function
pointers via the FUNCTION_ALIGNMENT macro (gcc/default.h). This does
not appear to change the default alignment on any architecture, but
it causes a failure in i386/gcc.target/i386/attr-aligned.c when
requesting a smaller alignment which is then silently ignored.

I am not sure what the best approach is, but my preference
would be to remove the lang hook and the FUNCTION_ALIGNMENT
logic which will also fix the test case (the requested
alignment will be applied).

I would then instead add a warning (or error?) which triggers
only with -fno-trampolines if the user requests an alignment
which is too small for this mechanism to work.

Does this sound reasonable?

Best,
Martin


> > 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;
> 
> Extraneous parens here too.
> 
> >  }
> >  
> >  /* 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;
> > +
> 
> And here too.
> 
> 
> > 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"]
> > +}
> > +
> 
> I think this needs documenting in sourcebuild.texi.   Look for
> "Effective-target" to find the appropriate section.
> 
> 
> Jeff


diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index 3c5e3ed4329..1702a9cd3cd 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,15 @@
+2018-12-10  Martin Uecker  <martin.uec...@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.
+       * doc/sourcebuild.texi (target attributes): Document new
+       "notrampolines" effective target keyword.
+
 2018-12-10  Uros Bizjak  <ubiz...@gmail.com>
 
        PR target/88418
diff --git a/gcc/ada/ChangeLog b/gcc/ada/ChangeLog
index b48c757b816..eb14239dc62 100644
--- a/gcc/ada/ChangeLog
+++ b/gcc/ada/ChangeLog
@@ -1,3 +1,8 @@
+2018-12-10  Martin Uecker  <martin.uec...@med.uni-goettingen.de>
+
+       * gcc-interface/trans.c (Attribute_to_gnu): Add check for
+       flag_trampolines.
+
 2018-12-03  Gary Dismukes  <dismu...@adacore.com>
 
        * sem_aux.adb (Object_Type_Has_Constrained_Partial_View): Return
diff --git a/gcc/ada/gcc-interface/trans.c b/gcc/ada/gcc-interface/trans.c
index 4c066c02421..97ce5ebd3d2 100644
--- a/gcc/ada/gcc-interface/trans.c
+++ b/gcc/ada/gcc-interface/trans.c
@@ -2239,7 +2239,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
@@ -5020,7 +5021,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 9bbfe76040b..4b4bd432304 100644
--- a/gcc/c/ChangeLog
+++ b/gcc/c/ChangeLog
@@ -1,3 +1,10 @@
+2018-12-10  Martin Uecker  <martin.uec...@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-12-08  Segher Boessenkool  <seg...@kernel.crashing.org>
 
        * c-parser (c_parser_asm_statement) [RID_INLINE]: Delete stray line
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 1a897273088..b14033064f3 100644
--- a/gcc/c/c-typeck.c
+++ b/gcc/c/c-typeck.c
@@ -1913,7 +1913,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
@@ -3135,6 +3141,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 98c6377d78f..8b6742fd0ba 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 08bffdf2c2d..5d441316ad2 100644
--- a/gcc/common.opt
+++ b/gcc/common.opt
@@ -2546,7 +2546,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 dd262b60d88..92f63ecc19d 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -14036,7 +14036,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/doc/sourcebuild.texi b/gcc/doc/sourcebuild.texi
index 1204a546c29..86ccd8ca16f 100644
--- a/gcc/doc/sourcebuild.texi
+++ b/gcc/doc/sourcebuild.texi
@@ -2162,6 +2162,9 @@ Target supports Newlib.
 GCC was configured with @code{--enable-newlib-nano-formatted-io}, which reduces
 the code size of Newlib formatted I/O functions.
 
+@item notrampolines
+Target supports option @option{-fno-trampolines}.
+
 @item pow10
 Target provides @code{pow10} function.
 
diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog
index 35b4fe14c2e..8f205396e21 100644
--- a/gcc/testsuite/ChangeLog
+++ b/gcc/testsuite/ChangeLog
@@ -1,3 +1,9 @@
+2018-12-10  Martin Uecker  <martin.uec...@med.uni-goettingen.de>
+
+       * gcc.dg/trampoline-2.c: New test.
+       * lib/target-supports.exp 
+       (check_effective_target_notrampolines): New.
+
 2018-12-10  Steven G. Kargl  <ka...@gcc.gnu.org>
 
        PR fortran/88269
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.uec...@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 5026c5906cd..44f3b0c1f37 100644
--- a/gcc/testsuite/lib/target-supports.exp
+++ b/gcc/testsuite/lib/target-supports.exp
@@ -924,6 +924,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 ab20cd98969..765ce347172 100644
--- a/gcc/toplev.c
+++ b/gcc/toplev.c
@@ -1698,6 +1698,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 0ad469ada49..eb9bccb7a9d 100644
--- a/gcc/tree-nested.c
+++ b/gcc/tree-nested.c
@@ -2544,7 +2544,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);

Reply via email to