sra: Avoid totally scalarizing overallping field_decls (PR93667)

Message ID ri6mu9fj3dz.fsf@suse.cz
State New
Headers show
Series
  • sra: Avoid totally scalarizing overallping field_decls (PR93667)
Related show

Commit Message

Martin Jambor Feb. 18, 2020, 3:56 p.m.
Hi,

[[no_unique_address]] C++ attribute can cause two fields of a
RECORD_TYPE overlap, which currently confuses the totally scalarizing
code into creating invalid access tree.  For GCC 10, I'd like to
simply disable total scalarization of types where this happens.

For GCC 11 I'll write down a TODO item to enable total scalarization
of cases like this where the problematic fields are basically empty -
despite having a non-zero size - i.e. when they are just RECORD_TYPEs
without any data fields.

Bootstrapped and tested on x86_64, OK for trunk?

Thanks,

Martin

2020-02-14  Martin Jambor  <mjambor@suse.cz>

	PR tree-optimization/93667
	* tree-sra.c (scalarizable_type_p): Return false if record fields
	do not follow wach other.

	testsuite/
	* g++.dg/tree-ssa/pr93667.C: New test.
---
 gcc/ChangeLog                           |  6 ++++++
 gcc/testsuite/ChangeLog                 |  5 +++++
 gcc/testsuite/g++.dg/tree-ssa/pr93667.C | 11 +++++++++++
 gcc/tree-sra.c                          | 14 ++++++++++++++
 4 files changed, 36 insertions(+)
 create mode 100644 gcc/testsuite/g++.dg/tree-ssa/pr93667.C

-- 
2.25.0

Comments

Richard Biener Feb. 18, 2020, 4:45 p.m. | #1
On February 18, 2020 4:56:08 PM GMT+01:00, Martin Jambor <mjambor@suse.cz> wrote:
>Hi,

>

>[[no_unique_address]] C++ attribute can cause two fields of a

>RECORD_TYPE overlap, which currently confuses the totally scalarizing

>code into creating invalid access tree.  For GCC 10, I'd like to

>simply disable total scalarization of types where this happens.

>

>For GCC 11 I'll write down a TODO item to enable total scalarization

>of cases like this where the problematic fields are basically empty -

>despite having a non-zero size - i.e. when they are just RECORD_TYPEs

>without any data fields.

>

>Bootstrapped and tested on x86_64, OK for trunk?


Ok. 

Richard. 

>Thanks,

>

>Martin

>

>2020-02-14  Martin Jambor  <mjambor@suse.cz>

>

>	PR tree-optimization/93667

>	* tree-sra.c (scalarizable_type_p): Return false if record fields

>	do not follow wach other.

>

>	testsuite/

>	* g++.dg/tree-ssa/pr93667.C: New test.

>---

> gcc/ChangeLog                           |  6 ++++++

> gcc/testsuite/ChangeLog                 |  5 +++++

> gcc/testsuite/g++.dg/tree-ssa/pr93667.C | 11 +++++++++++

> gcc/tree-sra.c                          | 14 ++++++++++++++

> 4 files changed, 36 insertions(+)

> create mode 100644 gcc/testsuite/g++.dg/tree-ssa/pr93667.C

>

>diff --git a/gcc/testsuite/g++.dg/tree-ssa/pr93667.C

>b/gcc/testsuite/g++.dg/tree-ssa/pr93667.C

>new file mode 100644

>index 00000000000..d875f53d9ec

>--- /dev/null

>+++ b/gcc/testsuite/g++.dg/tree-ssa/pr93667.C

>@@ -0,0 +1,11 @@

>+// { dg-do compile }

>+// { dg-options "-O2 -std=c++2a" } */

>+

>+struct a {};

>+struct b { [[no_unique_address]] a aq; };

>+struct c {

>+  int d;

>+  [[no_unique_address]] b e;

>+};

>+c f() {return {};}

>+void g() { f(); }

>diff --git a/gcc/tree-sra.c b/gcc/tree-sra.c

>index 0cfac0a8192..4c7d651e6b9 100644

>--- a/gcc/tree-sra.c

>+++ b/gcc/tree-sra.c

>@@ -958,6 +958,9 @@ scalarizable_type_p (tree type, bool const_decl)

>   if (type_contains_placeholder_p (type))

>     return false;

> 

>+  bool have_predecessor_field = false;

>+  HOST_WIDE_INT prev_pos = 0;

>+

>   switch (TREE_CODE (type))

>   {

>   case RECORD_TYPE:

>@@ -966,6 +969,17 @@ scalarizable_type_p (tree type, bool const_decl)

> 	{

> 	  tree ft = TREE_TYPE (fld);

> 

>+	  if (zerop (DECL_SIZE (fld)))

>+	    continue;

>+

>+	  HOST_WIDE_INT pos = int_bit_position (fld);

>+	  if (have_predecessor_field

>+	      && pos <= prev_pos)

>+	    return false;

>+

>+	  have_predecessor_field = true;

>+	  prev_pos = pos;

>+

> 	  if (DECL_BIT_FIELD (fld))

> 	    return false;

>

Patch

diff --git a/gcc/testsuite/g++.dg/tree-ssa/pr93667.C b/gcc/testsuite/g++.dg/tree-ssa/pr93667.C
new file mode 100644
index 00000000000..d875f53d9ec
--- /dev/null
+++ b/gcc/testsuite/g++.dg/tree-ssa/pr93667.C
@@ -0,0 +1,11 @@ 
+// { dg-do compile }
+// { dg-options "-O2 -std=c++2a" } */
+
+struct a {};
+struct b { [[no_unique_address]] a aq; };
+struct c {
+  int d;
+  [[no_unique_address]] b e;
+};
+c f() {return {};}
+void g() { f(); }
diff --git a/gcc/tree-sra.c b/gcc/tree-sra.c
index 0cfac0a8192..4c7d651e6b9 100644
--- a/gcc/tree-sra.c
+++ b/gcc/tree-sra.c
@@ -958,6 +958,9 @@  scalarizable_type_p (tree type, bool const_decl)
   if (type_contains_placeholder_p (type))
     return false;
 
+  bool have_predecessor_field = false;
+  HOST_WIDE_INT prev_pos = 0;
+
   switch (TREE_CODE (type))
   {
   case RECORD_TYPE:
@@ -966,6 +969,17 @@  scalarizable_type_p (tree type, bool const_decl)
 	{
 	  tree ft = TREE_TYPE (fld);
 
+	  if (zerop (DECL_SIZE (fld)))
+	    continue;
+
+	  HOST_WIDE_INT pos = int_bit_position (fld);
+	  if (have_predecessor_field
+	      && pos <= prev_pos)
+	    return false;
+
+	  have_predecessor_field = true;
+	  prev_pos = pos;
+
 	  if (DECL_BIT_FIELD (fld))
 	    return false;