Handle OPAQUE_TYPE specially in verify_type [PR106833]

Message ID adbbc237-ef38-07c5-060f-72afdfabb8aa@linux.ibm.com
State New, archived
Headers
Series Handle OPAQUE_TYPE specially in verify_type [PR106833] |

Commit Message

Kewen.Lin Sept. 8, 2022, 5:53 a.m. UTC
  Hi,

As PR106833 shows, cv-qualified opaque type can cause ICE
during LTO.  It exposes that we missd to handle OPAQUE_TYPE
well in type verification.  As Richi pointed out, also
assuming that target will always define TYPE_MAIN_VARIANT
and TYPE_CANONICAL for opaque type, this patch is to check
both are OPAQUE_TYPE_P.  Besides, it also checks the only
available size and alignment information as well as type
mode for TYPE_MAIN_VARIANT.

Bootstrapped and regtested on powerpc64-linux-gnu P7 and
powerpc64le-linux-gnu P9 and P10.

Is it ok for trunk?

BR,
Kewen
-----
	PR middle-end/106833

gcc/ChangeLog:

	* tree.cc (verify_opaque_type): New function.
	(verify_match): New macro.
	(verify_type): Call verify_opaque_type for OPAQUE_TYPE.

gcc/testsuite/ChangeLog:

	* gcc.target/powerpc/pr106833.c: New test.
---
 gcc/testsuite/gcc.target/powerpc/pr106833.c | 14 +++++++
 gcc/tree.cc                                 | 45 ++++++++++++++++++++-
 2 files changed, 58 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/gcc.target/powerpc/pr106833.c

--
2.27.0
  

Comments

Richard Biener Sept. 8, 2022, 7:36 a.m. UTC | #1
> Am 08.09.2022 um 07:53 schrieb Kewen.Lin <linkw@linux.ibm.com>:
> 
> Hi,
> 
> As PR106833 shows, cv-qualified opaque type can cause ICE
> during LTO.  It exposes that we missd to handle OPAQUE_TYPE
> well in type verification.  As Richi pointed out, also
> assuming that target will always define TYPE_MAIN_VARIANT
> and TYPE_CANONICAL for opaque type, this patch is to check
> both are OPAQUE_TYPE_P.  Besides, it also checks the only
> available size and alignment information as well as type
> mode for TYPE_MAIN_VARIANT.
> 
> Bootstrapped and regtested on powerpc64-linux-gnu P7 and
> powerpc64le-linux-gnu P9 and P10.
> 
> Is it ok for trunk?
> 
> BR,
> Kewen
> -----
>    PR middle-end/106833
> 
> gcc/ChangeLog:
> 
>    * tree.cc (verify_opaque_type): New function.
>    (verify_match): New macro.
>    (verify_type): Call verify_opaque_type for OPAQUE_TYPE.
> 
> gcc/testsuite/ChangeLog:
> 
>    * gcc.target/powerpc/pr106833.c: New test.
> ---
> gcc/testsuite/gcc.target/powerpc/pr106833.c | 14 +++++++
> gcc/tree.cc                                 | 45 ++++++++++++++++++++-
> 2 files changed, 58 insertions(+), 1 deletion(-)
> create mode 100644 gcc/testsuite/gcc.target/powerpc/pr106833.c
> 
> diff --git a/gcc/testsuite/gcc.target/powerpc/pr106833.c b/gcc/testsuite/gcc.target/powerpc/pr106833.c
> new file mode 100644
> index 00000000000..968d75184ff
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/powerpc/pr106833.c
> @@ -0,0 +1,14 @@
> +/* { dg-do link } */
> +/* { dg-require-effective-target power10_ok } */
> +/* { dg-require-effective-target lto } */
> +/* { dg-options "-flto -mdejagnu-cpu=power10" } */
> +
> +/* Verify there is no ICE in LTO mode.  */
> +
> +int main ()
> +{
> +  float *b;
> +  const __vector_quad c;
> +  __builtin_mma_disassemble_acc (b, &c);
> +  return 0;
> +}
> diff --git a/gcc/tree.cc b/gcc/tree.cc
> index fed1434d141..e67caa8f85d 100644
> --- a/gcc/tree.cc
> +++ b/gcc/tree.cc
> @@ -13670,6 +13670,42 @@ gimple_canonical_types_compatible_p (const_tree t1, const_tree t2,
>     }
> }
> 
> +/* For OPAQUE_TYPE T, it has only size and alignment information, so verify
> +   these properties of T match TV which is the main variant of T.  Also
> +   verify the type of TC, which is the canonical of T, is OPAQUE_TYPE.  */
> +
> +static void
> +verify_opaque_type (const_tree t, tree tv, tree tc)
> +{
> +  gcc_assert (OPAQUE_TYPE_P (t));
> +  gcc_assert (tv && tv == TYPE_MAIN_VARIANT (tv));
> +  gcc_assert (tc && tc == TYPE_CANONICAL (tc));
> +
> +#define verify_match(flag, t, tv)                                              \
> +  do                                                                           \
> +    {                                                                          \
> +      if (flag (t) != flag (tv))                                               \
> +    {                                                                      \
> +      error ("opaque type differs by %s", #flag);                          \
> +      debug_tree (tv);                                                     \
> +    }                                                                      \
> +    }                                                                          \
> +  while (false)
> +
> +  if (t != tv)
> +    {
> +      verify_match (TREE_CODE, t, tv);
> +      verify_match (TYPE_MODE, t, tv);
> +      verify_match (TYPE_SIZE, t, tv);

TYPE_SIZE is a tree, you should probably 
Compare this with operand_equal_p.  It’s 
Not documented to be a constant size?
Thus some VLA vector mode might be allowed ( a poly_int size), BLKmode
Is ruled out(?), the docs say we have
‚An MODE_Opaque‘ here but I don’t see
This being verified?

The macro makes this a bit unworldly
For the only benefit of elaborate diagnostic
Which I think isn’t really necessary

> +      verify_match (TYPE_ALIGN, t, tv);
> +      verify_match (TYPE_USER_ALIGN, t, tv);
> +    }
> +
> +  if (t != tc)
> +    verify_match (TREE_CODE, t, tc);
> +#undef verify_match
> +}
> +
> /* Verify type T.  */
> 
> void
> @@ -13677,6 +13713,14 @@ verify_type (const_tree t)
> {
>   bool error_found = false;
>   tree mv = TYPE_MAIN_VARIANT (t);
> +  tree ct = TYPE_CANONICAL (t);
> +
> +  if (OPAQUE_TYPE_P (t))
> +    {
> +      verify_opaque_type (t, mv, ct);
> +      return;
> +    }
> +
>   if (!mv)
>     {
>       error ("main variant is not defined");
> @@ -13691,7 +13735,6 @@ verify_type (const_tree t)
>   else if (t != mv && !verify_type_variant (t, mv))
>     error_found = true;
> 
> -  tree ct = TYPE_CANONICAL (t);
>   if (!ct)
>     ;
>   else if (TYPE_CANONICAL (ct) != ct)
> --
> 2.27.0
>
  

Patch

diff --git a/gcc/testsuite/gcc.target/powerpc/pr106833.c b/gcc/testsuite/gcc.target/powerpc/pr106833.c
new file mode 100644
index 00000000000..968d75184ff
--- /dev/null
+++ b/gcc/testsuite/gcc.target/powerpc/pr106833.c
@@ -0,0 +1,14 @@ 
+/* { dg-do link } */
+/* { dg-require-effective-target power10_ok } */
+/* { dg-require-effective-target lto } */
+/* { dg-options "-flto -mdejagnu-cpu=power10" } */
+
+/* Verify there is no ICE in LTO mode.  */
+
+int main ()
+{
+  float *b;
+  const __vector_quad c;
+  __builtin_mma_disassemble_acc (b, &c);
+  return 0;
+}
diff --git a/gcc/tree.cc b/gcc/tree.cc
index fed1434d141..e67caa8f85d 100644
--- a/gcc/tree.cc
+++ b/gcc/tree.cc
@@ -13670,6 +13670,42 @@  gimple_canonical_types_compatible_p (const_tree t1, const_tree t2,
     }
 }

+/* For OPAQUE_TYPE T, it has only size and alignment information, so verify
+   these properties of T match TV which is the main variant of T.  Also
+   verify the type of TC, which is the canonical of T, is OPAQUE_TYPE.  */
+
+static void
+verify_opaque_type (const_tree t, tree tv, tree tc)
+{
+  gcc_assert (OPAQUE_TYPE_P (t));
+  gcc_assert (tv && tv == TYPE_MAIN_VARIANT (tv));
+  gcc_assert (tc && tc == TYPE_CANONICAL (tc));
+
+#define verify_match(flag, t, tv)                                              \
+  do                                                                           \
+    {                                                                          \
+      if (flag (t) != flag (tv))                                               \
+	{                                                                      \
+	  error ("opaque type differs by %s", #flag);                          \
+	  debug_tree (tv);                                                     \
+	}                                                                      \
+    }                                                                          \
+  while (false)
+
+  if (t != tv)
+    {
+      verify_match (TREE_CODE, t, tv);
+      verify_match (TYPE_MODE, t, tv);
+      verify_match (TYPE_SIZE, t, tv);
+      verify_match (TYPE_ALIGN, t, tv);
+      verify_match (TYPE_USER_ALIGN, t, tv);
+    }
+
+  if (t != tc)
+    verify_match (TREE_CODE, t, tc);
+#undef verify_match
+}
+
 /* Verify type T.  */

 void
@@ -13677,6 +13713,14 @@  verify_type (const_tree t)
 {
   bool error_found = false;
   tree mv = TYPE_MAIN_VARIANT (t);
+  tree ct = TYPE_CANONICAL (t);
+
+  if (OPAQUE_TYPE_P (t))
+    {
+      verify_opaque_type (t, mv, ct);
+      return;
+    }
+
   if (!mv)
     {
       error ("main variant is not defined");
@@ -13691,7 +13735,6 @@  verify_type (const_tree t)
   else if (t != mv && !verify_type_variant (t, mv))
     error_found = true;

-  tree ct = TYPE_CANONICAL (t);
   if (!ct)
     ;
   else if (TYPE_CANONICAL (ct) != ct)