Re: [PATCH] Fix thunk expansion (PR ipa/64896)

[Yvan Roux <yvan dot roux at linaro dot org>]

Hi,


>>>         PR ipa/65236
>>>         * cgraphunit.c (cgraph_node::expand_thunk): Enable return slot
>>>         opt.
>
> This bugfix adds ipa-icf-6.C test which failed on 4.9 branch as
> ipa-icf is not backported on that branch.  Is the bugfix still
> relevant and we can dropped the testcase ?
>
>>>         PR ipa/64813
>>>         * cgraphunit.c (cgraph_node::expand_thunk): Do not create
>>>         a return value for call to a function that is noreturn.
>>>
>>>         PR ipa/63595
>>>         * cgraphunit.c (cgraph_node::expand_thunk): DECL_BY_REFERENCE
>>>         is correctly handled for thunks created by IPA ICF.
>>>
>>>         PR ipa/63587
>>>         * cgraphunit.c (cgraph_node::expand_thunk): Only VAR_DECLs are put
>>>         to local declarations.
>>>         * function.c (add_local_decl): Implementation moved from header
>>>         file, assert introduced for tree type.
>>>         * function.h: Likewise.

Here is the two patches that backport PR ipa/63587 and PR ipa/64813
fixes in 4.9 branch.  The 2 others introduce test cases that check
ipa-icf pass dumps, so I'm not sure if the code has to be backported.

bootstrapped/regtested on x86_64 and cross-compiled/regtested on
aarch64-linux-gnu
arm-linux-gnueabihf
armeb-linux-gnueabihf
i686-linux-gnu

Ok for 4.9 ?

Thanks
Yvan

----- PR 63587 -----
gcc/
2015-03-11  Yvan Roux  <yvan.roux@linaro.org>

    Backport from trunk r216841.
    2014-10-29  Martin Liska  <mliska@suse.cz>

    PR ipa/63587
    * cgraphunit.c (cgraph_node::expand_thunk): Only VAR_DECLs are put
    to local declarations.
    * function.c (add_local_decl): Implementation moved from header
    file, assert introduced for tree type.
    * function.h: Likewise.

gcc/testsuite/
2015-03-11  Yvan Roux  <yvan.roux@linaro.org>

    Backport from trunk r216841.
    2014-10-29  Martin Liska  <mliska@suse.cz>

    PR ipa/63587
    * g++.dg/ipa/pr63587-1.C: New test.
    * g++.dg/ipa/pr63587-2.C: New test.

----- PR 64813 -----
2015-03-11  Yvan Roux  <yvan.roux@linaro.org>

    Backport from trunk r220616.
    2015-02-11  Martin Liska  <mliska@suse.cz>

    PR ipa/64813
    * cgraphunit.c (cgraph_node::expand_thunk): Do not create
    a return value for call to a function that is noreturn.

diff --git a/gcc/cgraphunit.c b/gcc/cgraphunit.c
index 130fc0d..27016ad 100644
--- a/gcc/cgraphunit.c
+++ b/gcc/cgraphunit.c
@@ -1575,7 +1575,9 @@ expand_thunk (struct cgraph_node *node, bool output_asm_thunks)
 	      if (aggregate_value_p (resdecl, TREE_TYPE (thunk_fndecl)))
 		{
 		  restmp = resdecl;
-		  add_local_decl (cfun, restmp);
+
+	      if (TREE_CODE (restmp) == VAR_DECL)
+		    add_local_decl (cfun, restmp);
 		  BLOCK_VARS (DECL_INITIAL (current_function_decl)) = restmp;
 		}
 	      else
diff --git a/gcc/function.c b/gcc/function.c
index 1a8682b..b377667 100644
--- a/gcc/function.c
+++ b/gcc/function.c
@@ -7193,6 +7193,15 @@ match_asm_constraints_1 (rtx insn, rtx *p_sets, int noutputs)
     df_insn_rescan (insn);
 }
 
+/* Add the decl D to the local_decls list of FUN.  */
+
+void
+add_local_decl (struct function *fun, tree d)
+{
+  gcc_assert (TREE_CODE (d) == VAR_DECL);
+  vec_safe_push (fun->local_decls, d);
+}
+
 static unsigned
 rest_of_match_asm_constraints (void)
 {
diff --git a/gcc/function.h b/gcc/function.h
index 38a0fc4..fd4639c 100644
--- a/gcc/function.h
+++ b/gcc/function.h
@@ -674,11 +674,7 @@ struct GTY(()) function {
 
 /* Add the decl D to the local_decls list of FUN.  */
 
-static inline void
-add_local_decl (struct function *fun, tree d)
-{
-  vec_safe_push (fun->local_decls, d);
-}
+void add_local_decl (struct function *fun, tree d);
 
 #define FOR_EACH_LOCAL_DECL(FUN, I, D)		\
   FOR_EACH_VEC_SAFE_ELT_REVERSE ((FUN)->local_decls, I, D)
diff --git a/gcc/testsuite/g++.dg/ipa/pr63587-1.C b/gcc/testsuite/g++.dg/ipa/pr63587-1.C
new file mode 100644
index 0000000..cbf872e
--- /dev/null
+++ b/gcc/testsuite/g++.dg/ipa/pr63587-1.C
@@ -0,0 +1,92 @@
+// PR ipa/63587
+// { dg-do compile { target c++11 } }
+// { dg-options "-O2 -fno-strict-aliasing" }
+
+template <class> struct A
+{
+};
+template <typename> struct B
+{
+  template <typename> struct C;
+};
+class D;
+template <typename> class F;
+struct G
+{
+  void operator()(const D &, D);
+};
+class D
+{
+public:
+  D (int);
+};
+struct H
+{
+  H (int);
+};
+template <typename _Key, typename, typename, typename _Compare, typename>
+class I
+{
+  typedef _Key key_type;
+  template <typename _Key_compare> struct J
+  {
+    _Key_compare _M_key_compare;
+  };
+  J<_Compare> _M_impl;
+
+public:
+  A<int> _M_get_insert_unique_pos (const key_type &);
+  A<int> _M_get_insert_hint_unique_pos (H &);
+  template <typename... _Args> int _M_emplace_hint_unique (H, _Args &&...);
+};
+template <typename _Key, typename _Tp, typename _Compare = G,
+	  typename _Alloc = F<A<_Tp> > >
+class K
+{
+  typedef _Key key_type;
+  typedef _Key value_type;
+  typedef typename B<_Alloc>::template C<value_type> _Pair_alloc_type;
+  I<key_type, value_type, int, _Compare, _Pair_alloc_type> _M_t;
+
+public:
+  void operator[](key_type)
+  {
+    _M_t._M_emplace_hint_unique (0);
+  }
+};
+template <typename _Key, typename _Val, typename _KeyOfValue,
+	  typename _Compare, typename _Alloc>
+A<int>
+I<_Key, _Val, _KeyOfValue, _Compare, _Alloc>::_M_get_insert_unique_pos (
+  const key_type &p1)
+{
+  _M_impl._M_key_compare (p1, 0);
+}
+template <typename _Key, typename _Val, typename _KeyOfValue,
+	  typename _Compare, typename _Alloc>
+A<int>
+I<_Key, _Val, _KeyOfValue, _Compare, _Alloc>::_M_get_insert_hint_unique_pos (
+  H &)
+{
+  _M_get_insert_unique_pos (0);
+}
+template <typename _Key, typename _Val, typename _KeyOfValue,
+	  typename _Compare, typename _Alloc>
+template <typename... _Args>
+int
+I<_Key, _Val, _KeyOfValue, _Compare, _Alloc>::_M_emplace_hint_unique (
+  H p1, _Args &&...)
+{
+  _M_get_insert_hint_unique_pos (p1);
+}
+namespace {
+struct L;
+}
+void
+fn1 ()
+{
+  K<D, L> a;
+  a[0];
+  K<D, int> b;
+  b[0];
+}
diff --git a/gcc/testsuite/g++.dg/ipa/pr63587-2.C b/gcc/testsuite/g++.dg/ipa/pr63587-2.C
new file mode 100644
index 0000000..f31c5bd
--- /dev/null
+++ b/gcc/testsuite/g++.dg/ipa/pr63587-2.C
@@ -0,0 +1,250 @@
+// PR ipa/63587
+// { dg-do compile { target c++11 } }
+// { dg-options "-O2" }
+
+namespace boost {
+class basic_cstring
+{
+public:
+  basic_cstring (char *);
+};
+template <typename> struct identity
+{
+};
+struct make_identity;
+struct function_buffer
+{
+};
+template <typename FunctionObj> struct function_obj_invoker0
+{
+  static int
+  invoke (function_buffer &)
+  {
+    FunctionObj f;
+    f ();
+  }
+};
+template <typename FunctionObj> struct get_function_obj_invoker0
+{
+  typedef function_obj_invoker0<FunctionObj> type;
+};
+template <typename FunctionObj> struct apply
+{
+  typedef typename get_function_obj_invoker0<FunctionObj>::type invoker_type;
+};
+struct basic_vtable0
+{
+  typedef int (*invoker_type)(function_buffer &);
+  template <typename F> void assign_to (F, function_buffer);
+  invoker_type invoker;
+};
+class function0
+{
+public:
+  template <typename Functor> function0 (Functor)
+  {
+    typedef typename apply<Functor>::invoker_type invoker_type;
+    basic_vtable0 stored_vtable { invoker_type::invoke };
+    stored_vtable.assign_to (0, functor);
+  }
+  function_buffer functor;
+};
+class function : function0
+{
+public:
+  template <typename Functor> function (Functor f) : function0 (f) {}
+};
+class test_unit_generator
+{
+};
+class test_case
+{
+public:
+  test_case (basic_cstring, basic_cstring, int, function);
+};
+struct auto_test_unit_registrar
+{
+  auto_test_unit_registrar (test_unit_generator);
+};
+template <typename F> F unwrap (F, int);
+struct for_each_impl
+{
+  template <typename Iterator, typename LastIterator, typename TransformFunc,
+	    typename F>
+  static void
+  execute (Iterator, LastIterator, TransformFunc, F f)
+  {
+    identity<char> __trans_tmp_1;
+    unwrap (f, 0)(__trans_tmp_1);
+  }
+};
+template <typename, typename, typename F>
+void
+for_each (F f)
+{
+  for_each_impl::execute (0, 0, 0, f);
+}
+template <typename TestCaseTemplate> class test_case_template_invoker
+{
+public:
+  void operator()()
+  {
+    TestCaseTemplate::run (0);
+  }
+};
+template <typename Generator, typename TestCaseTemplate>
+struct generate_test_case_4_type
+{
+  generate_test_case_4_type (basic_cstring, basic_cstring, int, Generator G)
+    : m_test_case_name (0), m_test_case_file (0), m_holder (G)
+  {
+  }
+  template <typename TestType> void operator()(identity<TestType>)
+  {
+    test_case (0, 0, 0, test_case_template_invoker<TestCaseTemplate> ());
+  }
+  basic_cstring m_test_case_name;
+  basic_cstring m_test_case_file;
+  Generator m_holder;
+};
+template <typename TestCaseTemplate>
+class template_test_case_gen : public test_unit_generator
+{
+public:
+  template_test_case_gen (basic_cstring, basic_cstring, int)
+  {
+    for_each<int, make_identity> (
+      generate_test_case_4_type<template_test_case_gen, TestCaseTemplate> (
+	0, 0, 0, *this));
+  }
+};
+class attribute_name
+{
+  int m_id;
+
+public:
+  attribute_name (char);
+};
+template <typename> struct term;
+namespace exprns_ {
+template <typename> struct expr;
+}
+using exprns_::expr;
+template <typename T> struct Trans_NS_proto_terminal
+{
+  typedef expr<term<T> > type;
+};
+namespace exprns_ {
+template <typename Arg0> struct expr<term<Arg0> >
+{
+  Arg0 child0;
+};
+}
+template <typename Expr> struct actor
+{
+  typename Trans_NS_proto_terminal<Expr>::type proto_expr_;
+};
+template <template <typename> class Actor = actor> struct terminal
+{
+  typedef Actor<int> type;
+};
+namespace log {
+struct to_log_fun
+{
+};
+class value_extractor;
+template <typename, typename = value_extractor, typename = void,
+	  template <typename> class = actor>
+class attribute_actor;
+class attribute_terminal
+{
+public:
+  attribute_name m_name;
+  attribute_name
+  get_name ()
+  {
+    return m_name;
+  }
+};
+template <typename, typename, typename, template <typename> class ActorT>
+class attribute_actor : ActorT<attribute_terminal>
+{
+public:
+  typedef int value_type;
+  attribute_name
+  get_name ()
+  {
+    return this->proto_expr_.child0.get_name ();
+  }
+};
+template <typename AttributeValueT>
+attribute_actor<AttributeValueT> attr (attribute_name);
+terminal<>::type stream;
+template <typename LeftT, typename ImplT> class attribute_output_terminal
+{
+public:
+  template <typename U>
+  attribute_output_terminal (LeftT, attribute_name, ImplT, U);
+};
+template <typename LeftT> struct make_output_expression
+{
+  typedef attribute_output_terminal<LeftT, to_log_fun> type;
+  template <typename RightT>
+  static type
+  make (LeftT left, RightT &right)
+  {
+    type (left, right.get_name (), to_log_fun (), 0);
+  }
+};
+template <typename, typename RightT, typename = typename RightT::value_type>
+struct make_output_actor;
+template <template <typename> class ActorT, typename LeftExprT,
+	  typename RightT, typename ValueT>
+struct make_output_actor<ActorT<LeftExprT>, RightT, ValueT>
+{
+  typedef make_output_expression<ActorT<LeftExprT> > make_expression;
+  typedef ActorT<typename make_expression::type> type;
+  static type
+  make (ActorT<LeftExprT> left, RightT &right)
+  {
+    type { make_expression::make (left, right) };
+  }
+};
+template <typename LeftExprT, typename T, typename FallbackPolicyT,
+	  typename TagT>
+typename make_output_actor<actor<LeftExprT>, attribute_actor<TagT> >::type
+operator<<(actor<LeftExprT> left,
+	   attribute_actor<T, FallbackPolicyT, TagT> right)
+{
+  make_output_actor<actor<LeftExprT>, attribute_actor<T> >::make (left, right);
+}
+}
+}
+namespace logging = boost::log;
+namespace expr = logging;
+namespace {
+class my_class;
+}
+template <typename> struct default_formatting
+{
+  void test_method ();
+};
+struct default_formatting_invoker
+{
+  static void
+  run (void *)
+  {
+    default_formatting<int> t;
+    t.test_method ();
+  }
+};
+boost::auto_test_unit_registrar default_formatting_registrar56 (
+  boost::template_test_case_gen<default_formatting_invoker> (0, 0, 0));
+template <typename CharT>
+void
+default_formatting<CharT>::test_method ()
+{
+  expr::stream << expr::attr<my_class> (0);
+  expr::stream << expr::attr<int> (0) << expr::attr<int> (0)
+	       << expr::attr<int> (0);
+}
-- 
1.9.1

diff --git a/gcc/cgraphunit.c b/gcc/cgraphunit.c
index 130fc0d..2fcb84c 100644
--- a/gcc/cgraphunit.c
+++ b/gcc/cgraphunit.c
@@ -1532,6 +1532,7 @@ expand_thunk (struct cgraph_node *node, bool output_asm_thunks)
 
       gimple call;
       gimple ret;
+      bool alias_is_noreturn = TREE_THIS_VOLATILE (alias);
 
       if (in_lto_p)
 	cgraph_get_body (node);
@@ -1566,7 +1567,7 @@ expand_thunk (struct cgraph_node *node, bool output_asm_thunks)
       bsi = gsi_start_bb (bb);
 
       /* Build call to the function being thunked.  */
-      if (!VOID_TYPE_P (restype))
+      if (!VOID_TYPE_P (restype) && !alias_is_noreturn)
 	{
 	  if (DECL_BY_REFERENCE (resdecl))
 	    restmp = gimple_fold_indirect_ref (resdecl);
@@ -1610,14 +1611,14 @@ expand_thunk (struct cgraph_node *node, bool output_asm_thunks)
       call = gimple_build_call_vec (build_fold_addr_expr_loc (0, alias), vargs);
       node->callees->call_stmt = call;
       gimple_call_set_from_thunk (call, true);
-      if (restmp)
+      if (restmp && !alias_is_noreturn)
 	{
           gimple_call_set_lhs (call, restmp);
 	  gcc_assert (useless_type_conversion_p (TREE_TYPE (restmp),
 						 TREE_TYPE (TREE_TYPE (alias))));
 	}
       gsi_insert_after (&bsi, call, GSI_NEW_STMT);
-      if (!(gimple_call_flags (call) & ECF_NORETURN))
+      if (!alias_is_noreturn)
 	{
 	  if (restmp && !this_adjusting
 	      && (fixed_offset || virtual_offset))
-- 
1.9.1