diff mbox series

expr: Unbreak build of mesa [PR96194]

Message ID 20200714133735.GV2363@tucnak
State New
Headers show
Series expr: Unbreak build of mesa [PR96194] | expand

Commit Message

Jakub Jelinek July 14, 2020, 1:37 p.m. UTC 
Hi!

On Tue, Jul 07, 2020 at 09:04:40AM +0200, Richard Biener via Gcc-patches wrote:
> > The store to the whole of each volatile object was picked apart
> > like there had been an individual assignment to each of the
> > fields.  Reads were added as part of that; see PR for details.
> > The reads from volatile memory were a clear bug; individual
> > stores questionable.  A separate patch clarifies the docs.
> >
> > Tested x86_64-linux, powerpc64le-linux and cris-elf.
> > Ok to commit?  Backport to gcc-10?
> 
> OK for both.

This breaks building of mesa on both the trunk and 10 branch.

The problem is that the middle-end may never create temporaries of non-POD
(TREE_ADDRESSABLE) types, those can be only created when the language says
so and thus only the FE is allowed to create those.

This patch just reverts the behavior to what we used to do before for the
stores to volatile non-PODs.  Perhaps we want to do something else, but
definitely we can't create temporaries of the non-POD type.  It is up to
discussions on what should happen in those cases.

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

2020-07-14  Jakub Jelinek  <jakub@redhat.com>

	PR middle-end/96194
	* expr.c (expand_constructor): Don't create temporary for store to
	volatile MEM if exp has an addressable type.

	* g++.dg/opt/pr96194.C: New test.



	Jakub

Comments

Richard Biener July 14, 2020, 1:57 p.m. UTC | #1 
On July 14, 2020 3:37:35 PM GMT+02:00, Jakub Jelinek <jakub@redhat.com> wrote:
>Hi!
>
>On Tue, Jul 07, 2020 at 09:04:40AM +0200, Richard Biener via
>Gcc-patches wrote:
>> > The store to the whole of each volatile object was picked apart
>> > like there had been an individual assignment to each of the
>> > fields.  Reads were added as part of that; see PR for details.
>> > The reads from volatile memory were a clear bug; individual
>> > stores questionable.  A separate patch clarifies the docs.
>> >
>> > Tested x86_64-linux, powerpc64le-linux and cris-elf.
>> > Ok to commit?  Backport to gcc-10?
>> 
>> OK for both.
>
>This breaks building of mesa on both the trunk and 10 branch.
>
>The problem is that the middle-end may never create temporaries of
>non-POD
>(TREE_ADDRESSABLE) types, those can be only created when the language
>says
>so and thus only the FE is allowed to create those.
>
>This patch just reverts the behavior to what we used to do before for
>the
>stores to volatile non-PODs.  Perhaps we want to do something else, but
>definitely we can't create temporaries of the non-POD type.  It is up
>to
>discussions on what should happen in those cases.
>
>Bootstrapped/regtested on x86_64-linux and i686-linux, ok for
>trunk/10.2?

OK. 

Richard. 

>2020-07-14  Jakub Jelinek  <jakub@redhat.com>
>
>	PR middle-end/96194
>	* expr.c (expand_constructor): Don't create temporary for store to
>	volatile MEM if exp has an addressable type.
>
>	* g++.dg/opt/pr96194.C: New test.
>
>--- gcc/expr.c.jj	2020-07-13 19:09:33.173872178 +0200
>+++ gcc/expr.c	2020-07-14 13:07:26.228801996 +0200
>@@ -8382,7 +8382,9 @@ expand_constructor (tree exp, rtx target
>      || GET_CODE (target) == PARALLEL || modifier == EXPAND_STACK_PARM
>       /* Also make a temporary if the store is to volatile memory, to
> 	 avoid individual accesses to aggregate members.  */
>-      || (GET_CODE (target) == MEM && MEM_VOLATILE_P (target)))
>+      || (GET_CODE (target) == MEM
>+	  && MEM_VOLATILE_P (target)
>+	  && !TREE_ADDRESSABLE (TREE_TYPE (exp))))
>     {
>       if (avoid_temp_mem)
> 	return NULL_RTX;
>--- gcc/testsuite/g++.dg/opt/pr96194.C.jj	2020-07-14 13:15:32.185733939
>+0200
>+++ gcc/testsuite/g++.dg/opt/pr96194.C	2020-07-14 13:16:16.033096197
>+0200
>@@ -0,0 +1,21 @@
>+// PR middle-end/96194
>+// { dg-do compile }
>+// { dg-options "-O2" }
>+
>+#include <new>
>+
>+struct A { ~A (); };
>+struct B : A { float e[64]; };
>+
>+B *
>+foo ()
>+{
>+  return new ((void *) 0) B ();
>+}
>+
>+B *
>+bar (void *x, bool y)
>+{
>+  void *p = y ? x : (void *) 0;
>+  return new (p) B ();
>+}
>
>
>	Jakub
diff mbox series

Patch

--- gcc/expr.c.jj	2020-07-13 19:09:33.173872178 +0200
+++ gcc/expr.c	2020-07-14 13:07:26.228801996 +0200
@@ -8382,7 +8382,9 @@  expand_constructor (tree exp, rtx target
       || GET_CODE (target) == PARALLEL || modifier == EXPAND_STACK_PARM
       /* Also make a temporary if the store is to volatile memory, to
 	 avoid individual accesses to aggregate members.  */
-      || (GET_CODE (target) == MEM && MEM_VOLATILE_P (target)))
+      || (GET_CODE (target) == MEM
+	  && MEM_VOLATILE_P (target)
+	  && !TREE_ADDRESSABLE (TREE_TYPE (exp))))
     {
       if (avoid_temp_mem)
 	return NULL_RTX;
--- gcc/testsuite/g++.dg/opt/pr96194.C.jj	2020-07-14 13:15:32.185733939 +0200
+++ gcc/testsuite/g++.dg/opt/pr96194.C	2020-07-14 13:16:16.033096197 +0200
@@ -0,0 +1,21 @@ 
+// PR middle-end/96194
+// { dg-do compile }
+// { dg-options "-O2" }
+
+#include <new>
+
+struct A { ~A (); };
+struct B : A { float e[64]; };
+
+B *
+foo ()
+{
+  return new ((void *) 0) B ();
+}
+
+B *
+bar (void *x, bool y)
+{
+  void *p = y ? x : (void *) 0;
+  return new (p) B ();
+}