diff mbox series

c++: Fix coroutines on targetm.cxx.cdtor_return_this targets [PR99459]

Message ID 20210309095158.GL745611@tucnak
State New
Headers show
Series c++: Fix coroutines on targetm.cxx.cdtor_return_this targets [PR99459] | expand

Commit Message

Jakub Jelinek March 9, 2021, 9:51 a.m. UTC
Hi!

The r11-7528 build_co_await changes broke coroutines on arm*-linux-gnuabi,
2780 ^FAIL.*coroutines/ in total.
The problem is that arm is targetm.cxx.cdtor_return_this target where
both ctors and dtors in the ABI return this pointer rather than
void, and build_new_method_call_1 does:
              else if (call != error_mark_node
                       && DECL_DESTRUCTOR_P (cand->fn)
                       && !VOID_TYPE_P (TREE_TYPE (call)))
                /* An explicit call of the form "x->~X()" has type
                   "void".  However, on platforms where destructors
                   return "this" (i.e., those where
                   targetm.cxx.cdtor_returns_this is true), such calls
                   will appear to have a return value of pointer type
                   to the low-level call machinery.  We do not want to
                   change the low-level machinery, since we want to be
                   able to optimize "delete f()" on such platforms as
                   "operator delete(~X(f()))" (rather than generating
                   "t = f(), ~X(t), operator delete (t)").  */
                call = build_nop (void_type_node, call);
The new code in build_co_await relies on build_special_member_call
returned expression being a CALL_EXPR, but due to the build_nop
in there it is a NOP_EXPR around the CALL_EXPR.  It can't be stripped
with STRIP_NOPS because void has different mode from the pointer mode.

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

2021-03-09  Jakub Jelinek  <jakub@redhat.com>

	PR c++/99459
	* coroutines.cc (build_co_await): Look through NOP_EXPRs in
	build_special_member_call return value to find the CALL_EXPR.


	Jakub

Comments

Nathan Sidwell March 9, 2021, 1:01 p.m. UTC | #1
On 3/9/21 4:51 AM, Jakub Jelinek wrote:
> Hi!
> 
> The r11-7528 build_co_await changes broke coroutines on arm*-linux-gnuabi,
> 2780 ^FAIL.*coroutines/ in total.
> The problem is that arm is targetm.cxx.cdtor_return_this target where
> both ctors and dtors in the ABI return this pointer rather than
> void, and build_new_method_call_1 does:
>                else if (call != error_mark_node
>                         && DECL_DESTRUCTOR_P (cand->fn)
>                         && !VOID_TYPE_P (TREE_TYPE (call)))
>                  /* An explicit call of the form "x->~X()" has type
>                     "void".  However, on platforms where destructors
>                     return "this" (i.e., those where
>                     targetm.cxx.cdtor_returns_this is true), such calls
>                     will appear to have a return value of pointer type
>                     to the low-level call machinery.  We do not want to
>                     change the low-level machinery, since we want to be
>                     able to optimize "delete f()" on such platforms as
>                     "operator delete(~X(f()))" (rather than generating
>                     "t = f(), ~X(t), operator delete (t)").  */
>                  call = build_nop (void_type_node, call);
> The new code in build_co_await relies on build_special_member_call
> returned expression being a CALL_EXPR, but due to the build_nop
> in there it is a NOP_EXPR around the CALL_EXPR.  It can't be stripped
> with STRIP_NOPS because void has different mode from the pointer mode.
> 
> Bootstrapped/regtested on armv7hl-linux-gnueabi and x86_64-linux, ok for trunk?

thanks for digging into this.  Looks good, but could you take the 
opportunity to rewrite the conditionals to a single

if (dummy) { ... do the non-null things ... }

?
> 
> 2021-03-09  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR c++/99459
> 	* coroutines.cc (build_co_await): Look through NOP_EXPRs in
> 	build_special_member_call return value to find the CALL_EXPR.
> 
> --- gcc/cp/coroutines.cc.jj	2021-03-05 21:51:48.671185716 +0100
> +++ gcc/cp/coroutines.cc	2021-03-08 10:53:13.187959339 +0100
> @@ -868,6 +868,8 @@ build_co_await (location_t loc, tree a,
>   		= build_special_member_call (a, complete_dtor_identifier,
>   					     NULL, a_type, LOOKUP_NORMAL,
>   					     tf_none);
> +	      if (dummy && CONVERT_EXPR_P (dummy))
> +		dummy = TREE_OPERAND (dummy, 0);
>   	      dummy = dummy ? TREE_OPERAND (CALL_EXPR_FN (dummy), 0)
>   			    : NULL_TREE;
>   	      if (dummy && coro_diagnose_throwing_fn (dummy))
> @@ -1031,6 +1033,8 @@ build_co_await (location_t loc, tree a,
>   	    = build_special_member_call (e_proxy, complete_dtor_identifier,
>   					 NULL, o_type, LOOKUP_NORMAL,
>   					 tf_none);
> +	  if (dummy && CONVERT_EXPR_P (dummy))
> +	    dummy = TREE_OPERAND (dummy, 0);
>   	  dummy = dummy ? TREE_OPERAND (CALL_EXPR_FN (dummy), 0)
>   			: NULL_TREE;
>   	  if (dummy && coro_diagnose_throwing_fn (dummy))
> 
> 	Jakub
>
diff mbox series

Patch

--- gcc/cp/coroutines.cc.jj	2021-03-05 21:51:48.671185716 +0100
+++ gcc/cp/coroutines.cc	2021-03-08 10:53:13.187959339 +0100
@@ -868,6 +868,8 @@  build_co_await (location_t loc, tree a,
 		= build_special_member_call (a, complete_dtor_identifier,
 					     NULL, a_type, LOOKUP_NORMAL,
 					     tf_none);
+	      if (dummy && CONVERT_EXPR_P (dummy))
+		dummy = TREE_OPERAND (dummy, 0);
 	      dummy = dummy ? TREE_OPERAND (CALL_EXPR_FN (dummy), 0)
 			    : NULL_TREE;
 	      if (dummy && coro_diagnose_throwing_fn (dummy))
@@ -1031,6 +1033,8 @@  build_co_await (location_t loc, tree a,
 	    = build_special_member_call (e_proxy, complete_dtor_identifier,
 					 NULL, o_type, LOOKUP_NORMAL,
 					 tf_none);
+	  if (dummy && CONVERT_EXPR_P (dummy))
+	    dummy = TREE_OPERAND (dummy, 0);
 	  dummy = dummy ? TREE_OPERAND (CALL_EXPR_FN (dummy), 0)
 			: NULL_TREE;
 	  if (dummy && coro_diagnose_throwing_fn (dummy))