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

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

[Jakub Jelinek]

Hi!

As discussed in the PR, for functions that return an aggregate that is not
aggregate_value_p (i.e. returned in registers), using RESULT_DECL is
undesirable, that's not what we normally emit for user code.

So, this patch instead uses a temporary, which is optimized right now as
much as similar user written code.

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

2015-02-06  Jakub Jelinek  <jakub@redhat.com>

	PR ipa/64896
	* cgraphunit.c (cgraph_node::expand_thunk): If
	restype is not is_gimple_reg_type nor the thunk_fndecl
	returns aggregate_value_p, set restmp to a temporary variable
	instead of resdecl.

	* g++.dg/ipa/pr64896.C: New test.

--- gcc/cgraphunit.c.jj	2015-01-29 21:38:21.000000000 +0100
+++ gcc/cgraphunit.c	2015-02-06 13:18:44.870527405 +0100
@@ -1609,11 +1609,16 @@ cgraph_node::expand_thunk (bool output_a
 	    }
 	  else if (!is_gimple_reg_type (restype))
 	    {
-	      restmp = resdecl;
+	      if (aggregate_value_p (resdecl, TREE_TYPE (thunk_fndecl)))
+		{
+		  restmp = resdecl;
 
-	      if (TREE_CODE (restmp) == VAR_DECL)
-		add_local_decl (cfun, restmp);
-	      BLOCK_VARS (DECL_INITIAL (current_function_decl)) = restmp;
+		  if (TREE_CODE (restmp) == VAR_DECL)
+		    add_local_decl (cfun, restmp);
+		  BLOCK_VARS (DECL_INITIAL (current_function_decl)) = restmp;
+		}
+	      else
+		restmp = create_tmp_var (restype, "retval");
 	    }
 	  else
 	    restmp = create_tmp_reg (restype, "retval");
--- gcc/testsuite/g++.dg/ipa/pr64896.C.jj	2015-02-06 13:36:30.076680258 +0100
+++ gcc/testsuite/g++.dg/ipa/pr64896.C	2015-02-06 13:36:13.000000000 +0100
@@ -0,0 +1,29 @@
+// PR ipa/64896
+// { dg-do compile }
+// { dg-options "-O2" }
+
+struct A { int a, b; };
+struct B { A c; int d; };
+struct C { virtual B fn1 () const; };
+struct D { B fn2 () const; int fn3 () const; C *fn4 () const; };
+
+int
+D::fn3 () const
+{
+  fn4 ()->fn1 ();
+}
+
+B
+D::fn2 () const
+{
+  return B ();
+}
+
+class F : C
+{
+  B
+  fn1 () const
+  {
+    return B ();
+  }
+};

	Jakub

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

[Jan Hubicka]

> Hi!
> 
> As discussed in the PR, for functions that return an aggregate that is not
> aggregate_value_p (i.e. returned in registers), using RESULT_DECL is
> undesirable, that's not what we normally emit for user code.
> 
> So, this patch instead uses a temporary, which is optimized right now as
> much as similar user written code.
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

OK,
thanks!
Honza
> 
> 2015-02-06  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR ipa/64896
> 	* cgraphunit.c (cgraph_node::expand_thunk): If
> 	restype is not is_gimple_reg_type nor the thunk_fndecl
> 	returns aggregate_value_p, set restmp to a temporary variable
> 	instead of resdecl.
> 
> 	* g++.dg/ipa/pr64896.C: New test.
> 
> --- gcc/cgraphunit.c.jj	2015-01-29 21:38:21.000000000 +0100
> +++ gcc/cgraphunit.c	2015-02-06 13:18:44.870527405 +0100
> @@ -1609,11 +1609,16 @@ cgraph_node::expand_thunk (bool output_a
>  	    }
>  	  else if (!is_gimple_reg_type (restype))
>  	    {
> -	      restmp = resdecl;
> +	      if (aggregate_value_p (resdecl, TREE_TYPE (thunk_fndecl)))
> +		{
> +		  restmp = resdecl;
>  
> -	      if (TREE_CODE (restmp) == VAR_DECL)
> -		add_local_decl (cfun, restmp);
> -	      BLOCK_VARS (DECL_INITIAL (current_function_decl)) = restmp;
> +		  if (TREE_CODE (restmp) == VAR_DECL)
> +		    add_local_decl (cfun, restmp);
> +		  BLOCK_VARS (DECL_INITIAL (current_function_decl)) = restmp;
> +		}
> +	      else
> +		restmp = create_tmp_var (restype, "retval");
>  	    }
>  	  else
>  	    restmp = create_tmp_reg (restype, "retval");
> --- gcc/testsuite/g++.dg/ipa/pr64896.C.jj	2015-02-06 13:36:30.076680258 +0100
> +++ gcc/testsuite/g++.dg/ipa/pr64896.C	2015-02-06 13:36:13.000000000 +0100
> @@ -0,0 +1,29 @@
> +// PR ipa/64896
> +// { dg-do compile }
> +// { dg-options "-O2" }
> +
> +struct A { int a, b; };
> +struct B { A c; int d; };
> +struct C { virtual B fn1 () const; };
> +struct D { B fn2 () const; int fn3 () const; C *fn4 () const; };
> +
> +int
> +D::fn3 () const
> +{
> +  fn4 ()->fn1 ();
> +}
> +
> +B
> +D::fn2 () const
> +{
> +  return B ();
> +}
> +
> +class F : C
> +{
> +  B
> +  fn1 () const
> +  {
> +    return B ();
> +  }
> +};
> 
> 	Jakub

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

[Yvan Roux]

[-- Attachment #1: Type: text/plain, Size: 1135 bytes --]

Hi,

As added in the PR, this issue is also present on 4.9 branch and
affects at least arm-linux-gnueabihf target (as reported in PR61207).

I've backported it in the 4.9 branch with the attached patch.  The
difference with the trunk code is due the code introduced by PR63587
fix (I didn't checked on power7, on which the PR was initially
reported, but I didn't managed to reproduce the issue for arm targets
on 4.9 branch).

Boostrapped on x86_64, and tested on arm/aarch64 targets (regression
testing is ongoing). is ok for 4.9 branch when  validation is done ?


Thanks
Yvan

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

    Backport from trunk r220489.
    2015-02-06  Jakub Jelinek  <jakub@redhat.com>

    PR ipa/64896
    * cgraphunit.c (cgraph_node::expand_thunk): If
    restype is not is_gimple_reg_type nor the thunk_fndecl
    returns aggregate_value_p, set restmp to a temporary variable
    instead of resdecl.

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

    Backport from trunk r220489.
    2015-02-06  Jakub Jelinek  <jakub@redhat.com>

    PR ipa/64896
    * g++.dg/ipa/pr64896.C: New test.

[-- Attachment #2: pr64896.diff --]
[-- Type: text/plain, Size: 1366 bytes --]

diff --git a/gcc/cgraphunit.c b/gcc/cgraphunit.c
index 8f57607..130fc0d 100644
--- a/gcc/cgraphunit.c
+++ b/gcc/cgraphunit.c
@@ -1572,9 +1572,14 @@ expand_thunk (struct cgraph_node *node, bool output_asm_thunks)
 	    restmp = gimple_fold_indirect_ref (resdecl);
 	  else if (!is_gimple_reg_type (restype))
 	    {
-	      restmp = resdecl;
-	      add_local_decl (cfun, restmp);
-	      BLOCK_VARS (DECL_INITIAL (current_function_decl)) = restmp;
+	      if (aggregate_value_p (resdecl, TREE_TYPE (thunk_fndecl)))
+		{
+		  restmp = resdecl;
+		  add_local_decl (cfun, restmp);
+		  BLOCK_VARS (DECL_INITIAL (current_function_decl)) = restmp;
+		}
+	      else
+		restmp = create_tmp_var (restype, "retval");
 	    }
 	  else
 	    restmp = create_tmp_reg (restype, "retval");
diff --git a/gcc/testsuite/g++.dg/ipa/pr64896.C b/gcc/testsuite/g++.dg/ipa/pr64896.C
new file mode 100644
index 0000000..0a78220
--- /dev/null
+++ b/gcc/testsuite/g++.dg/ipa/pr64896.C
@@ -0,0 +1,29 @@
+// PR ipa/64896
+// { dg-do compile }
+// { dg-options "-O2" }
+
+struct A { int a, b; };
+struct B { A c; int d; };
+struct C { virtual B fn1 () const; };
+struct D { B fn2 () const; int fn3 () const; C *fn4 () const; };
+
+int
+D::fn3 () const
+{
+  fn4 ()->fn1 ();
+}
+
+B
+D::fn2 () const
+{
+  return B ();
+}
+
+class F : C
+{
+  B
+  fn1 () const
+  {
+    return B ();
+  }
+};

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

[Yvan Roux]

Hi

On 9 March 2015 at 17:07, Yvan Roux <yvan.roux@linaro.org> wrote:
> Hi,
>
> As added in the PR, this issue is also present on 4.9 branch and
> affects at least arm-linux-gnueabihf target (as reported in PR61207).
>
> I've backported it in the 4.9 branch with the attached patch.  The
> difference with the trunk code is due the code introduced by PR63587
> fix (I didn't checked on power7, on which the PR was initially
> reported, but I didn't managed to reproduce the issue for arm targets
> on 4.9 branch).
>
> Boostrapped on x86_64, and tested on arm/aarch64 targets (regression
> testing is ongoing). is ok for 4.9 branch when  validation is done ?

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

> Thanks
> Yvan
>
> gcc/
> 2015-03-09  Yvan Roux  <yvan.roux@linaro.org>
>
>     Backport from trunk r220489.
>     2015-02-06  Jakub Jelinek  <jakub@redhat.com>
>
>     PR ipa/64896
>     * cgraphunit.c (cgraph_node::expand_thunk): If
>     restype is not is_gimple_reg_type nor the thunk_fndecl
>     returns aggregate_value_p, set restmp to a temporary variable
>     instead of resdecl.
>
> gcc/testsuite/
> 2015-03-09  Yvan Roux  <yvan.roux@linaro.org>
>
>     Backport from trunk r220489.
>     2015-02-06  Jakub Jelinek  <jakub@redhat.com>
>
>     PR ipa/64896
>     * g++.dg/ipa/pr64896.C: New test.

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

[Jan Hubicka]

> Hi
> 
> On 9 March 2015 at 17:07, Yvan Roux <yvan.roux@linaro.org> wrote:
> > Hi,
> >
> > As added in the PR, this issue is also present on 4.9 branch and
> > affects at least arm-linux-gnueabihf target (as reported in PR61207).
> >
> > I've backported it in the 4.9 branch with the attached patch.  The
> > difference with the trunk code is due the code introduced by PR63587
> > fix (I didn't checked on power7, on which the PR was initially
> > reported, but I didn't managed to reproduce the issue for arm targets
> > on 4.9 branch).
> >
> > Boostrapped on x86_64, and tested on arm/aarch64 targets (regression
> > testing is ongoing). is ok for 4.9 branch when  validation is done ?
> 
> So bootstrapped/regtested on x86_64 and cross-compiled/regtested on
> aarch64-linux-gnu
> arm-linux-gnueabihf
> armeb-linux-gnueabihf
> i686-linux-gnu

This is OK. note that cgraph_node::expand_thunk has gathered quite few
extra fixes that may be resonable for backporting.  Looking across
changes after ipa-icf was enabled I think we should look into
these:

        PR ipa/65236
        * cgraphunit.c (cgraph_node::expand_thunk): Enable return slot
        opt.

        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.

While these bugs was triggered by ipa-icf, they all IMO can be reproduced by
thunks on targets that do not define assembler thunks.
(most are about return values and those are not excercised on main targets with
MI thunks because covariant thunks always returns pointer)

Honza

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

[Yvan Roux]

On 10 March 2015 at 19:18, Jan Hubicka <hubicka@ucw.cz> wrote:
>> Hi
>>
>> On 9 March 2015 at 17:07, Yvan Roux <yvan.roux@linaro.org> wrote:
>> > Hi,
>> >
>> > As added in the PR, this issue is also present on 4.9 branch and
>> > affects at least arm-linux-gnueabihf target (as reported in PR61207).
>> >
>> > I've backported it in the 4.9 branch with the attached patch.  The
>> > difference with the trunk code is due the code introduced by PR63587
>> > fix (I didn't checked on power7, on which the PR was initially
>> > reported, but I didn't managed to reproduce the issue for arm targets
>> > on 4.9 branch).
>> >
>> > Boostrapped on x86_64, and tested on arm/aarch64 targets (regression
>> > testing is ongoing). is ok for 4.9 branch when  validation is done ?
>>
>> So bootstrapped/regtested on x86_64 and cross-compiled/regtested on
>> aarch64-linux-gnu
>> arm-linux-gnueabihf
>> armeb-linux-gnueabihf
>> i686-linux-gnu
>
> This is OK. note that cgraph_node::expand_thunk has gathered quite few
> extra fixes that may be resonable for backporting.  Looking across
> changes after ipa-icf was enabled I think we should look into
> these:
>
>         PR ipa/65236
>         * cgraphunit.c (cgraph_node::expand_thunk): Enable return slot
>         opt.
>
>         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.
>
> While these bugs was triggered by ipa-icf, they all IMO can be reproduced by
> thunks on targets that do not define assembler thunks.
> (most are about return values and those are not excercised on main targets with
> MI thunks because covariant thunks always returns pointer)

Thanks Honza.  I can backport all of them and pass the same validation
I did for this one if you want.

Yvan

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

[Yvan Roux]

Honza,

On 10 March 2015 at 20:09, Yvan Roux <yvan.roux@linaro.org> wrote:
> On 10 March 2015 at 19:18, Jan Hubicka <hubicka@ucw.cz> wrote:
>>> Hi
>>>
>>> On 9 March 2015 at 17:07, Yvan Roux <yvan.roux@linaro.org> wrote:
>>> > Hi,
>>> >
>>> > As added in the PR, this issue is also present on 4.9 branch and
>>> > affects at least arm-linux-gnueabihf target (as reported in PR61207).
>>> >
>>> > I've backported it in the 4.9 branch with the attached patch.  The
>>> > difference with the trunk code is due the code introduced by PR63587
>>> > fix (I didn't checked on power7, on which the PR was initially
>>> > reported, but I didn't managed to reproduce the issue for arm targets
>>> > on 4.9 branch).
>>> >
>>> > Boostrapped on x86_64, and tested on arm/aarch64 targets (regression
>>> > testing is ongoing). is ok for 4.9 branch when  validation is done ?
>>>
>>> So bootstrapped/regtested on x86_64 and cross-compiled/regtested on
>>> aarch64-linux-gnu
>>> arm-linux-gnueabihf
>>> armeb-linux-gnueabihf
>>> i686-linux-gnu
>>
>> This is OK. note that cgraph_node::expand_thunk has gathered quite few
>> extra fixes that may be resonable for backporting.  Looking across
>> changes after ipa-icf was enabled I think we should look into
>> these:
>>
>>         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.
>>
>> While these bugs was triggered by ipa-icf, they all IMO can be reproduced by
>> thunks on targets that do not define assembler thunks.
>> (most are about return values and those are not excercised on main targets with
>> MI thunks because covariant thunks always returns pointer)
>
> Thanks Honza.  I can backport all of them and pass the same validation
> I did for this one if you want.

The test introduced


> Yvan

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

[Yvan Roux]

[-- Attachment #1: Type: text/plain, Size: 2252 bytes --]

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.

[-- Attachment #2: pr63587.patch --]
[-- Type: text/x-patch, Size: 10119 bytes --]

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


[-- Attachment #3: pr64813.patch --]
[-- Type: text/x-patch, Size: 1430 bytes --]

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

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

[Yvan Roux]

Ping.

On 11 March 2015 at 16:38, Yvan Roux <yvan.roux@linaro.org> wrote:
> 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.

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

[Jan Hubicka]

> > 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.
OK,
thanks!
Honza