public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] ctf: Fix multi-dimentional array types ordering in CTF
@ 2024-02-28 15:03 Cupertino Miranda
  2024-03-04 18:00 ` [PATCH,V2] ctf: fix incorrect CTF for multi-dimensional array types Indu Bhagat
  0 siblings, 1 reply; 5+ messages in thread
From: Cupertino Miranda @ 2024-02-28 15:03 UTC (permalink / raw)
  To: gcc-patches
  Cc: jose.marchesi, david.faust, elena.zannoni, indu.bhagat,
	Cupertino Miranda

Hi everyone,

In order to facilitate reviewing, I include a copy of the function in
this email, since the code structure changes are too hard to analyse
in the patch itself.

Looking forward to your comments.

Regards,
Cupertino

=== Function changes ===

/* Generate CTF for an ARRAY_TYPE.
   C argument is used as the iterator for the recursive calls to
   gen_ctf_array_type. C is the current die within the recursion.
   When C is NULL, it means it is the first call to gen_ctf_array_type.
   C should always be NULL when called from other functions.  */

static ctf_id_t
gen_ctf_array_type (ctf_container_ref ctfc,
		    dw_die_ref array_type,
		    dw_die_ref c = NULL)
{
  int vector_type_p = get_AT_flag (array_type, DW_AT_GNU_vector);
  if (vector_type_p)
    return CTF_NULL_TYPEID;

  if (c == dw_get_die_child (array_type))
    {
      dw_die_ref array_elems_type = ctf_get_AT_type (array_type);
      return gen_ctf_type (ctfc, array_elems_type);
    }
  else
    {
      ctf_arinfo_t arinfo;
      ctf_id_t array_node_type_id = CTF_NULL_TYPEID;
      if (c == NULL)
	c = dw_get_die_child (array_type);

      c = dw_get_die_sib (c);

      ctf_id_t child_id = gen_ctf_array_type (ctfc, array_type, c);

      dw_die_ref array_index_type;
      uint32_t array_num_elements;

      if (dw_get_die_tag (c) == DW_TAG_subrange_type)
	{
	  dw_attr_node *upper_bound_at;

	  array_index_type = ctf_get_AT_type (c);

	  /* When DW_AT_upper_bound is used to specify the size of an
	     array in DWARF, it is usually an unsigned constant
	     specifying the upper bound index of the array.  However,
	     for unsized arrays, such as foo[] or bar[0],
	     DW_AT_upper_bound is a signed integer constant
	     instead.  */

	  upper_bound_at = get_AT (c, DW_AT_upper_bound);
	  if (upper_bound_at
	      && AT_class (upper_bound_at) == dw_val_class_unsigned_const)
	    /* This is the upper bound index.  */
	    array_num_elements = get_AT_unsigned (c, DW_AT_upper_bound) + 1;
	  else if (get_AT (c, DW_AT_count))
	    array_num_elements = get_AT_unsigned (c, DW_AT_count);
	  else
	    {
	      /* This is a VLA of some kind.  */
	      array_num_elements = 0;
	    }
	}
      else if (dw_get_die_tag (c) == DW_TAG_enumeration_type)
	{
	  array_index_type = 0;
	  array_num_elements = 0;
	  /* XXX writeme. */
	  gcc_assert (1);
	}
      else
	gcc_unreachable ();

      /* Ok, mount and register the array type.  Note how the array
	 type we register here is the type of the elements in
	 subsequent "dimensions", if there are any.  */

      arinfo.ctr_nelems = array_num_elements;
      if (array_index_type)
	arinfo.ctr_index = gen_ctf_type (ctfc, array_index_type);
      else
	arinfo.ctr_index = gen_ctf_type (ctfc, ctf_array_index_die);

      arinfo.ctr_contents = child_id;
      if (!ctf_type_exists (ctfc, c, &array_node_type_id))
	array_node_type_id = ctf_add_array (ctfc, CTF_ADD_ROOT, &arinfo,
					    c);
      return array_node_type_id;
    }
}

=== The patch ===

Multi-dimentional array types would be linked in reverse order to the
expected C standard ordering. In other words, CTF would define the type
of "char [a][b][c]" as if it was an array of "char [c][b][a]"
dimentions.

gcc/ChangeLog:

	* dwarf2ctf.cc (gen_ctf_array_type): Correct order in which CTF
	multi-dimentional array types are linked.

gcc/testsuite/ChangeLog:
	* gcc.dg/debug/ctf/ctf-array-6.c: Add test.
---
 gcc/dwarf2ctf.cc                             | 67 ++++++++------------
 gcc/testsuite/gcc.dg/debug/ctf/ctf-array-6.c | 14 ++++
 2 files changed, 41 insertions(+), 40 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/debug/ctf/ctf-array-6.c

diff --git a/gcc/dwarf2ctf.cc b/gcc/dwarf2ctf.cc
index 93e5619933fa..95ceca196217 100644
--- a/gcc/dwarf2ctf.cc
+++ b/gcc/dwarf2ctf.cc
@@ -349,42 +349,40 @@ gen_ctf_pointer_type (ctf_container_ref ctfc, dw_die_ref ptr_type)
   return ptr_type_id;
 }
 
-/* Generate CTF for an array type.  */
+/* Generate CTF for an ARRAY_TYPE.
+   C argument is used as the iterator for the recursive calls to
+   gen_ctf_array_type. C is the current die within the recursion.
+   When C is NULL, it means it is the first call to gen_ctf_array_type.
+   C should always be NULL when called from other functions.  */
 
 static ctf_id_t
-gen_ctf_array_type (ctf_container_ref ctfc, dw_die_ref array_type)
+gen_ctf_array_type (ctf_container_ref ctfc,
+		    dw_die_ref array_type,
+		    dw_die_ref c = NULL)
 {
-  dw_die_ref c;
-  ctf_id_t array_elems_type_id = CTF_NULL_TYPEID;
-
   int vector_type_p = get_AT_flag (array_type, DW_AT_GNU_vector);
   if (vector_type_p)
-    return array_elems_type_id;
-
-  dw_die_ref array_elems_type = ctf_get_AT_type (array_type);
+    return CTF_NULL_TYPEID;
 
-  /* First, register the type of the array elements if needed.  */
-  array_elems_type_id = gen_ctf_type (ctfc, array_elems_type);
+  if (c == dw_get_die_child (array_type))
+    {
+      dw_die_ref array_elems_type = ctf_get_AT_type (array_type);
+      return gen_ctf_type (ctfc, array_elems_type);
+    }
+  else
+    {
+      ctf_arinfo_t arinfo;
+      ctf_id_t array_node_type_id = CTF_NULL_TYPEID;
+      if (c == NULL)
+	c = dw_get_die_child (array_type);
 
-  /* DWARF array types pretend C supports multi-dimensional arrays.
-     So for the type int[N][M], the array type DIE contains two
-     subrange_type children, the first with upper bound N-1 and the
-     second with upper bound M-1.
+      c = dw_get_die_sib (c);
 
-     CTF, on the other hand, just encodes each array type in its own
-     array type CTF struct.  Therefore we have to iterate on the
-     children and create all the needed types.  */
+      ctf_id_t child_id = gen_ctf_array_type (ctfc, array_type, c);
 
-  c = dw_get_die_child (array_type);
-  gcc_assert (c);
-  do
-    {
-      ctf_arinfo_t arinfo;
       dw_die_ref array_index_type;
       uint32_t array_num_elements;
 
-      c = dw_get_die_sib (c);
-
       if (dw_get_die_tag (c) == DW_TAG_subrange_type)
 	{
 	  dw_attr_node *upper_bound_at;
@@ -431,23 +429,12 @@ gen_ctf_array_type (ctf_container_ref ctfc, dw_die_ref array_type)
       else
 	arinfo.ctr_index = gen_ctf_type (ctfc, ctf_array_index_die);
 
-      arinfo.ctr_contents = array_elems_type_id;
-      if (!ctf_type_exists (ctfc, c, &array_elems_type_id))
-	array_elems_type_id = ctf_add_array (ctfc, CTF_ADD_ROOT, &arinfo,
-					     c);
+      arinfo.ctr_contents = child_id;
+      if (!ctf_type_exists (ctfc, c, &array_node_type_id))
+	array_node_type_id = ctf_add_array (ctfc, CTF_ADD_ROOT, &arinfo,
+					    c);
+      return array_node_type_id;
     }
-  while (c != dw_get_die_child (array_type));
-
-#if 0
-  /* Type de-duplication.
-     Consult the ctfc_types hash again before adding the CTF array type because
-     there can be cases where an array_type type may have been added by the
-     gen_ctf_type call above.  */
-  if (!ctf_type_exists (ctfc, array_type, &type_id))
-    type_id = ctf_add_array (ctfc, CTF_ADD_ROOT, &arinfo, array_type);
-#endif
-
-  return array_elems_type_id;
 }
 
 /* Generate CTF for a typedef.  */
diff --git a/gcc/testsuite/gcc.dg/debug/ctf/ctf-array-6.c b/gcc/testsuite/gcc.dg/debug/ctf/ctf-array-6.c
new file mode 100644
index 000000000000..9391d6ba2eb5
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/debug/ctf/ctf-array-6.c
@@ -0,0 +1,14 @@
+/* CTF generation for multidimentional array.  */
+
+/* { dg-do compile )  */
+/* { dg-options "-O0 -gctf -dA" } */
+
+/* { dg-final { scan-assembler-times "\[\t \]+0x2\[\t \]+\[^\n\]*cta_nelems" 1 } } */
+/* { dg-final { scan-assembler-times "\[\t \]+0x3\[\t \]+\[^\n\]*cta_nelems" 1 } } */
+/* { dg-final { scan-assembler-times "\[\t \]+0x4\[\t \]+\[^\n\]*cta_nelems" 1 } } */
+
+/* { dg-final { scan-assembler-times "\t.4byte\t0x1\[\t \]+# cta_contents\[\\r\\n\]+\t.4byte\t0x2\[\t \]+# cta_index\[\\r\\n\]+\t.4byte\t0x4\[\t \]+# cta_nelems" 1 } } */
+/* { dg-final { scan-assembler-times "\t.4byte\t0x3\[\t \]+# cta_contents\[\\r\\n\]+\t.4byte\t0x2\[\t \]+# cta_index\[\\r\\n\]+\t.4byte\t0x3\[\t \]+# cta_nelems" 1 } } */
+/* { dg-final { scan-assembler-times "\t.4byte\t0x4\[\t \]+# cta_contents\[\\r\\n\]+\t.4byte\t0x2\[\t \]+# cta_index\[\\r\\n\]+\t.4byte\t0x2\[\t \]+# cta_nelems" 1 } } */
+
+int a[2][3][4];
-- 
2.30.2


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH,V2] ctf: fix incorrect CTF for multi-dimensional array types
  2024-02-28 15:03 [PATCH] ctf: Fix multi-dimentional array types ordering in CTF Cupertino Miranda
@ 2024-03-04 18:00 ` Indu Bhagat
  2024-03-04 19:46   ` David Faust
  2024-03-05  8:47   ` [PATCH, V3] " Indu Bhagat
  0 siblings, 2 replies; 5+ messages in thread
From: Indu Bhagat @ 2024-03-04 18:00 UTC (permalink / raw)
  To: gcc-patches; +Cc: david.faust, jose.marchesi, cupertino.miranda, Indu Bhagat

From: Cupertino Miranda <cupertino.miranda@oracle.com>

[Changes from V1]
  - Refactor the code a bit.
[End of changes from V1]

PR debug/114186

DWARF DIEs of type DW_TAG_subrange_type are linked together to represent
the information about the subsequent dimensions.  The CTF processing was
so far working through them in the opposite (incorrect) order.

While fixing the issue, refactor the code a bit for readability.

co-authored-By: Indu Bhagat <indu.bhagat@oracle.com>

gcc/
	PR debug/114186
	* dwarf2ctf.cc (gen_ctf_array_type): Invoke the ctf_add_array ()
	in the correct order of the dimensions.
        (gen_ctf_subrange_type): Refactor out handling of
	DW_TAG_subrange_type DIE to here.

gcc/testsuite/
	PR debug/114186
	* gcc.dg/debug/ctf/ctf-array-6.c: Add test.
---

Testing notes:

Regression tested on x86_64-linux-gnu default target.
Regression tested for target bpf-unknown-none (btf.exp, ctf.exp, bpf.exp).

---
 gcc/dwarf2ctf.cc                             | 153 +++++++++----------
 gcc/testsuite/gcc.dg/debug/ctf/ctf-array-6.c |  14 ++
 2 files changed, 84 insertions(+), 83 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/debug/ctf/ctf-array-6.c

diff --git a/gcc/dwarf2ctf.cc b/gcc/dwarf2ctf.cc
index dca86edfffa9..3985de115a79 100644
--- a/gcc/dwarf2ctf.cc
+++ b/gcc/dwarf2ctf.cc
@@ -349,105 +349,92 @@ gen_ctf_pointer_type (ctf_container_ref ctfc, dw_die_ref ptr_type)
   return ptr_type_id;
 }
 
-/* Generate CTF for an array type.  */
+/* Recursively generate CTF for array dimensions starting at DIE C (of type
+   DW_TAG_subrange_type) until DIE LAST (of type DW_TAG_subrange_type) is
+   reached.  ARRAY_ELEMS_TYPE_ID is base type for the array.  */
 
 static ctf_id_t
-gen_ctf_array_type (ctf_container_ref ctfc, dw_die_ref array_type)
+gen_ctf_subrange_type (ctf_container_ref ctfc, ctf_id_t array_elems_type_id,
+		       dw_die_ref c, dw_die_ref last)
 {
-  dw_die_ref c;
-  ctf_id_t array_elems_type_id = CTF_NULL_TYPEID;
+  ctf_arinfo_t arinfo;
+  ctf_id_t array_node_type_id = CTF_NULL_TYPEID;
+
+  dw_attr_node *upper_bound_at;
+  dw_die_ref array_index_type;
+  uint32_t array_num_elements;
+
+  /* When DW_AT_upper_bound is used to specify the size of an
+     array in DWARF, it is usually an unsigned constant
+     specifying the upper bound index of the array.  However,
+     for unsized arrays, such as foo[] or bar[0],
+     DW_AT_upper_bound is a signed integer constant
+     instead.  */
+
+  upper_bound_at = get_AT (c, DW_AT_upper_bound);
+  if (upper_bound_at
+      && AT_class (upper_bound_at) == dw_val_class_unsigned_const)
+    /* This is the ound index.  */
+    array_num_elements = get_AT_unsigned (c, DW_AT_upper_bound) + 1;
+  else if (get_AT (c, DW_AT_count))
+    array_num_elements = get_AT_unsigned (c, DW_AT_count);
+  else
+    {
+      /* This is a VLA of some kind.  */
+      array_num_elements = 0;
+    }
 
-  int vector_type_p = get_AT_flag (array_type, DW_AT_GNU_vector);
-  if (vector_type_p)
-    return array_elems_type_id;
+  /* Ok, mount and register the array type.  Note how the array
+     type we register here is the type of the elements in
+     subsequent "dimensions", if there are any.  */
+  arinfo.ctr_nelems = array_num_elements;
 
-  dw_die_ref array_elems_type = ctf_get_AT_type (array_type);
+  array_index_type = ctf_get_AT_type (c);
+  arinfo.ctr_index = gen_ctf_type (ctfc, array_index_type);
 
-  /* First, register the type of the array elements if needed.  */
-  array_elems_type_id = gen_ctf_type (ctfc, array_elems_type);
+  if (c == last)
+    arinfo.ctr_contents = array_elems_type_id;
+  else
+    arinfo.ctr_contents = gen_ctf_subrange_type (ctfc, array_elems_type_id,
+						 dw_get_die_sib (c), last);
 
-  /* DWARF array types pretend C supports multi-dimensional arrays.
-     So for the type int[N][M], the array type DIE contains two
-     subrange_type children, the first with upper bound N-1 and the
-     second with upper bound M-1.
+  if (!ctf_type_exists (ctfc, c, &array_node_type_id))
+    array_node_type_id = ctf_add_array (ctfc, CTF_ADD_ROOT, &arinfo, c);
 
-     CTF, on the other hand, just encodes each array type in its own
-     array type CTF struct.  Therefore we have to iterate on the
-     children and create all the needed types.  */
+  return array_node_type_id;
+}
 
-  c = dw_get_die_child (array_type);
-  gcc_assert (c);
-  do
-    {
-      ctf_arinfo_t arinfo;
-      dw_die_ref array_index_type;
-      uint32_t array_num_elements;
+/* Generate CTF for an ARRAY_TYPE.  */
 
-      c = dw_get_die_sib (c);
+static ctf_id_t
+gen_ctf_array_type (ctf_container_ref ctfc,
+		    dw_die_ref array_type)
+{
+  dw_die_ref first, last, array_elems_type;
+  ctf_id_t array_elems_type_id = CTF_NULL_TYPEID;
+  ctf_id_t array_type_id = CTF_NULL_TYPEID;
 
-      if (dw_get_die_tag (c) == DW_TAG_subrange_type)
-	{
-	  dw_attr_node *upper_bound_at;
-
-	  array_index_type = ctf_get_AT_type (c);
-
-	  /* When DW_AT_upper_bound is used to specify the size of an
-	     array in DWARF, it is usually an unsigned constant
-	     specifying the upper bound index of the array.  However,
-	     for unsized arrays, such as foo[] or bar[0],
-	     DW_AT_upper_bound is a signed integer constant
-	     instead.  */
-
-	  upper_bound_at = get_AT (c, DW_AT_upper_bound);
-	  if (upper_bound_at
-	      && AT_class (upper_bound_at) == dw_val_class_unsigned_const)
-	    /* This is the upper bound index.  */
-	    array_num_elements = get_AT_unsigned (c, DW_AT_upper_bound) + 1;
-	  else if (get_AT (c, DW_AT_count))
-	    array_num_elements = get_AT_unsigned (c, DW_AT_count);
-	  else
-	    {
-	      /* This is a VLA of some kind.  */
-	      array_num_elements = 0;
-	    }
-	}
-      else if (dw_get_die_tag (c) == DW_TAG_enumeration_type)
-	{
-	  array_index_type = 0;
-	  array_num_elements = 0;
-	  /* XXX writeme. */
-	  gcc_assert (1);
-	}
-      else
-	gcc_unreachable ();
+  int vector_type_p = get_AT_flag (array_type, DW_AT_GNU_vector);
+  if (vector_type_p)
+    return array_elems_type_id;
 
-      /* Ok, mount and register the array type.  Note how the array
-	 type we register here is the type of the elements in
-	 subsequent "dimensions", if there are any.  */
+  /* Find the first and last array dimension DIEs.  */
+  last = dw_get_die_child (array_type);
+  first = dw_get_die_sib (last);
 
-      arinfo.ctr_nelems = array_num_elements;
-      if (array_index_type)
-	arinfo.ctr_index = gen_ctf_type (ctfc, array_index_type);
-      else
-	arinfo.ctr_index = gen_ctf_type (ctfc, ctf_array_index_die);
+  /* Type de-duplication.
+     Consult the ctfc_types before adding CTF type for the first dimension.  */
+  if (!ctf_type_exists (ctfc, first, &array_type_id))
+    {
+      array_elems_type = ctf_get_AT_type (array_type);
+      /* First, register the type of the array elements if needed.  */
+      array_elems_type_id = gen_ctf_type (ctfc, array_elems_type);
 
-      arinfo.ctr_contents = array_elems_type_id;
-      if (!ctf_type_exists (ctfc, c, &array_elems_type_id))
-	array_elems_type_id = ctf_add_array (ctfc, CTF_ADD_ROOT, &arinfo,
-					     c);
+      array_type_id = gen_ctf_subrange_type (ctfc, array_elems_type_id, first,
+					     last);
     }
-  while (c != dw_get_die_child (array_type));
 
-#if 0
-  /* Type de-duplication.
-     Consult the ctfc_types hash again before adding the CTF array type because
-     there can be cases where an array_type type may have been added by the
-     gen_ctf_type call above.  */
-  if (!ctf_type_exists (ctfc, array_type, &type_id))
-    type_id = ctf_add_array (ctfc, CTF_ADD_ROOT, &arinfo, array_type);
-#endif
-
-  return array_elems_type_id;
+  return array_type_id;
 }
 
 /* Generate CTF for a typedef.  */
diff --git a/gcc/testsuite/gcc.dg/debug/ctf/ctf-array-6.c b/gcc/testsuite/gcc.dg/debug/ctf/ctf-array-6.c
new file mode 100644
index 000000000000..5ecbb049535d
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/debug/ctf/ctf-array-6.c
@@ -0,0 +1,14 @@
+/* CTF generation for multidimentional array.  */
+
+/* { dg-do compile )  */
+/* { dg-options "-O0 -gctf -dA" } */
+
+/* { dg-final { scan-assembler-times "\[\t \]+0x2\[\t \]+\[^\n\]*cta_nelems" 1 } } */
+/* { dg-final { scan-assembler-times "\[\t \]+0x3\[\t \]+\[^\n\]*cta_nelems" 1 } } */
+/* { dg-final { scan-assembler-times "\[\t \]+0x4\[\t \]+\[^\n\]*cta_nelems" 1 } } */
+
+/* { dg-final { scan-assembler-times "\t.\[^\t \]+\t0x1\[\t \]+# cta_contents\[\\r\\n\]+\t.\[^\t \]+\t0x2\[\t \]+# cta_index\[\\r\\n\]+\t.\[^\t \]+\t0x4\[\t \]+# cta_nelems" 1 } } */
+/* { dg-final { scan-assembler-times "\t.\[^\t \]+\t0x3\[\t \]+# cta_contents\[\\r\\n\]+\t.\[^\t \]+\t0x2\[\t \]+# cta_index\[\\r\\n\]+\t.\[^\t \]+\t0x3\[\t \]+# cta_nelems" 1 } } */
+/* { dg-final { scan-assembler-times "\t.\[^\t \]+\t0x4\[\t \]+# cta_contents\[\\r\\n\]+\t.\[^\t \]+\t0x2\[\t \]+# cta_index\[\\r\\n\]+\t.\[^\t \]+\t0x2\[\t \]+# cta_nelems" 1 } } */
+
+int a[2][3][4];
-- 
2.43.0


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH,V2] ctf: fix incorrect CTF for multi-dimensional array types
  2024-03-04 18:00 ` [PATCH,V2] ctf: fix incorrect CTF for multi-dimensional array types Indu Bhagat
@ 2024-03-04 19:46   ` David Faust
  2024-03-05  8:47   ` [PATCH, V3] " Indu Bhagat
  1 sibling, 0 replies; 5+ messages in thread
From: David Faust @ 2024-03-04 19:46 UTC (permalink / raw)
  To: Indu Bhagat, cupertino.miranda; +Cc: gcc-patches, jose.marchesi

Hi Indu, Cupertino,

On 3/4/24 10:00, Indu Bhagat wrote:
> From: Cupertino Miranda <cupertino.miranda@oracle.com>
> 
> [Changes from V1]
>   - Refactor the code a bit.
> [End of changes from V1]
> 
> PR debug/114186
> 
> DWARF DIEs of type DW_TAG_subrange_type are linked together to represent
> the information about the subsequent dimensions.  The CTF processing was
> so far working through them in the opposite (incorrect) order.
> 
> While fixing the issue, refactor the code a bit for readability.
> 
> co-authored-By: Indu Bhagat <indu.bhagat@oracle.com>

Thanks for the patch and refactor. I do find v2 easier to follow.
Two very minor typos in comments, noted inline below.

Otherwise, LGTM and OK.
Thanks!

> 
> gcc/
> 	PR debug/114186
> 	* dwarf2ctf.cc (gen_ctf_array_type): Invoke the ctf_add_array ()
> 	in the correct order of the dimensions.
>         (gen_ctf_subrange_type): Refactor out handling of
> 	DW_TAG_subrange_type DIE to here.
> 
> gcc/testsuite/
> 	PR debug/114186
> 	* gcc.dg/debug/ctf/ctf-array-6.c: Add test.
> ---
> 
> Testing notes:
> 
> Regression tested on x86_64-linux-gnu default target.
> Regression tested for target bpf-unknown-none (btf.exp, ctf.exp, bpf.exp).
> 
> ---
>  gcc/dwarf2ctf.cc                             | 153 +++++++++----------
>  gcc/testsuite/gcc.dg/debug/ctf/ctf-array-6.c |  14 ++
>  2 files changed, 84 insertions(+), 83 deletions(-)
>  create mode 100644 gcc/testsuite/gcc.dg/debug/ctf/ctf-array-6.c
> 
> diff --git a/gcc/dwarf2ctf.cc b/gcc/dwarf2ctf.cc
> index dca86edfffa9..3985de115a79 100644
> --- a/gcc/dwarf2ctf.cc
> +++ b/gcc/dwarf2ctf.cc
> @@ -349,105 +349,92 @@ gen_ctf_pointer_type (ctf_container_ref ctfc, dw_die_ref ptr_type)
>    return ptr_type_id;
>  }
>  
> -/* Generate CTF for an array type.  */
> +/* Recursively generate CTF for array dimensions starting at DIE C (of type
> +   DW_TAG_subrange_type) until DIE LAST (of type DW_TAG_subrange_type) is
> +   reached.  ARRAY_ELEMS_TYPE_ID is base type for the array.  */
>  
>  static ctf_id_t
> -gen_ctf_array_type (ctf_container_ref ctfc, dw_die_ref array_type)
> +gen_ctf_subrange_type (ctf_container_ref ctfc, ctf_id_t array_elems_type_id,
> +		       dw_die_ref c, dw_die_ref last)
>  {
> -  dw_die_ref c;
> -  ctf_id_t array_elems_type_id = CTF_NULL_TYPEID;
> +  ctf_arinfo_t arinfo;
> +  ctf_id_t array_node_type_id = CTF_NULL_TYPEID;
> +
> +  dw_attr_node *upper_bound_at;
> +  dw_die_ref array_index_type;
> +  uint32_t array_num_elements;
> +
> +  /* When DW_AT_upper_bound is used to specify the size of an
> +     array in DWARF, it is usually an unsigned constant
> +     specifying the upper bound index of the array.  However,
> +     for unsized arrays, such as foo[] or bar[0],
> +     DW_AT_upper_bound is a signed integer constant
> +     instead.  */
> +
> +  upper_bound_at = get_AT (c, DW_AT_upper_bound);
> +  if (upper_bound_at
> +      && AT_class (upper_bound_at) == dw_val_class_unsigned_const)
> +    /* This is the ound index.  */

typo, I guess this is meant to be "bound" ? 

> +    array_num_elements = get_AT_unsigned (c, DW_AT_upper_bound) + 1;
> +  else if (get_AT (c, DW_AT_count))
> +    array_num_elements = get_AT_unsigned (c, DW_AT_count);
> +  else
> +    {
> +      /* This is a VLA of some kind.  */
> +      array_num_elements = 0;
> +    }
>  
> -  int vector_type_p = get_AT_flag (array_type, DW_AT_GNU_vector);
> -  if (vector_type_p)
> -    return array_elems_type_id;
> +  /* Ok, mount and register the array type.  Note how the array
> +     type we register here is the type of the elements in
> +     subsequent "dimensions", if there are any.  */
> +  arinfo.ctr_nelems = array_num_elements;
>  
> -  dw_die_ref array_elems_type = ctf_get_AT_type (array_type);
> +  array_index_type = ctf_get_AT_type (c);
> +  arinfo.ctr_index = gen_ctf_type (ctfc, array_index_type);
>  
> -  /* First, register the type of the array elements if needed.  */
> -  array_elems_type_id = gen_ctf_type (ctfc, array_elems_type);
> +  if (c == last)
> +    arinfo.ctr_contents = array_elems_type_id;
> +  else
> +    arinfo.ctr_contents = gen_ctf_subrange_type (ctfc, array_elems_type_id,
> +						 dw_get_die_sib (c), last);
>  
> -  /* DWARF array types pretend C supports multi-dimensional arrays.
> -     So for the type int[N][M], the array type DIE contains two
> -     subrange_type children, the first with upper bound N-1 and the
> -     second with upper bound M-1.
> +  if (!ctf_type_exists (ctfc, c, &array_node_type_id))
> +    array_node_type_id = ctf_add_array (ctfc, CTF_ADD_ROOT, &arinfo, c);
>  
> -     CTF, on the other hand, just encodes each array type in its own
> -     array type CTF struct.  Therefore we have to iterate on the
> -     children and create all the needed types.  */
> +  return array_node_type_id;
> +}
>  
> -  c = dw_get_die_child (array_type);
> -  gcc_assert (c);
> -  do
> -    {
> -      ctf_arinfo_t arinfo;
> -      dw_die_ref array_index_type;
> -      uint32_t array_num_elements;
> +/* Generate CTF for an ARRAY_TYPE.  */
>  
> -      c = dw_get_die_sib (c);
> +static ctf_id_t
> +gen_ctf_array_type (ctf_container_ref ctfc,
> +		    dw_die_ref array_type)
> +{
> +  dw_die_ref first, last, array_elems_type;
> +  ctf_id_t array_elems_type_id = CTF_NULL_TYPEID;
> +  ctf_id_t array_type_id = CTF_NULL_TYPEID;
>  
> -      if (dw_get_die_tag (c) == DW_TAG_subrange_type)
> -	{
> -	  dw_attr_node *upper_bound_at;
> -
> -	  array_index_type = ctf_get_AT_type (c);
> -
> -	  /* When DW_AT_upper_bound is used to specify the size of an
> -	     array in DWARF, it is usually an unsigned constant
> -	     specifying the upper bound index of the array.  However,
> -	     for unsized arrays, such as foo[] or bar[0],
> -	     DW_AT_upper_bound is a signed integer constant
> -	     instead.  */
> -
> -	  upper_bound_at = get_AT (c, DW_AT_upper_bound);
> -	  if (upper_bound_at
> -	      && AT_class (upper_bound_at) == dw_val_class_unsigned_const)
> -	    /* This is the upper bound index.  */
> -	    array_num_elements = get_AT_unsigned (c, DW_AT_upper_bound) + 1;
> -	  else if (get_AT (c, DW_AT_count))
> -	    array_num_elements = get_AT_unsigned (c, DW_AT_count);
> -	  else
> -	    {
> -	      /* This is a VLA of some kind.  */
> -	      array_num_elements = 0;
> -	    }
> -	}
> -      else if (dw_get_die_tag (c) == DW_TAG_enumeration_type)
> -	{
> -	  array_index_type = 0;
> -	  array_num_elements = 0;
> -	  /* XXX writeme. */
> -	  gcc_assert (1);
> -	}
> -      else
> -	gcc_unreachable ();
> +  int vector_type_p = get_AT_flag (array_type, DW_AT_GNU_vector);
> +  if (vector_type_p)
> +    return array_elems_type_id;
>  
> -      /* Ok, mount and register the array type.  Note how the array
> -	 type we register here is the type of the elements in
> -	 subsequent "dimensions", if there are any.  */
> +  /* Find the first and last array dimension DIEs.  */
> +  last = dw_get_die_child (array_type);
> +  first = dw_get_die_sib (last);
>  
> -      arinfo.ctr_nelems = array_num_elements;
> -      if (array_index_type)
> -	arinfo.ctr_index = gen_ctf_type (ctfc, array_index_type);
> -      else
> -	arinfo.ctr_index = gen_ctf_type (ctfc, ctf_array_index_die);
> +  /* Type de-duplication.
> +     Consult the ctfc_types before adding CTF type for the first dimension.  */
> +  if (!ctf_type_exists (ctfc, first, &array_type_id))
> +    {
> +      array_elems_type = ctf_get_AT_type (array_type);
> +      /* First, register the type of the array elements if needed.  */
> +      array_elems_type_id = gen_ctf_type (ctfc, array_elems_type);
>  
> -      arinfo.ctr_contents = array_elems_type_id;
> -      if (!ctf_type_exists (ctfc, c, &array_elems_type_id))
> -	array_elems_type_id = ctf_add_array (ctfc, CTF_ADD_ROOT, &arinfo,
> -					     c);
> +      array_type_id = gen_ctf_subrange_type (ctfc, array_elems_type_id, first,
> +					     last);
>      }
> -  while (c != dw_get_die_child (array_type));
>  
> -#if 0
> -  /* Type de-duplication.
> -     Consult the ctfc_types hash again before adding the CTF array type because
> -     there can be cases where an array_type type may have been added by the
> -     gen_ctf_type call above.  */
> -  if (!ctf_type_exists (ctfc, array_type, &type_id))
> -    type_id = ctf_add_array (ctfc, CTF_ADD_ROOT, &arinfo, array_type);
> -#endif
> -
> -  return array_elems_type_id;
> +  return array_type_id;
>  }
>  
>  /* Generate CTF for a typedef.  */
> diff --git a/gcc/testsuite/gcc.dg/debug/ctf/ctf-array-6.c b/gcc/testsuite/gcc.dg/debug/ctf/ctf-array-6.c
> new file mode 100644
> index 000000000000..5ecbb049535d
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/debug/ctf/ctf-array-6.c
> @@ -0,0 +1,14 @@
> +/* CTF generation for multidimentional array.  */

nit: multidimensional (note 's' rather than 't')

> +
> +/* { dg-do compile )  */
> +/* { dg-options "-O0 -gctf -dA" } */
> +
> +/* { dg-final { scan-assembler-times "\[\t \]+0x2\[\t \]+\[^\n\]*cta_nelems" 1 } } */
> +/* { dg-final { scan-assembler-times "\[\t \]+0x3\[\t \]+\[^\n\]*cta_nelems" 1 } } */
> +/* { dg-final { scan-assembler-times "\[\t \]+0x4\[\t \]+\[^\n\]*cta_nelems" 1 } } */
> +
> +/* { dg-final { scan-assembler-times "\t.\[^\t \]+\t0x1\[\t \]+# cta_contents\[\\r\\n\]+\t.\[^\t \]+\t0x2\[\t \]+# cta_index\[\\r\\n\]+\t.\[^\t \]+\t0x4\[\t \]+# cta_nelems" 1 } } */
> +/* { dg-final { scan-assembler-times "\t.\[^\t \]+\t0x3\[\t \]+# cta_contents\[\\r\\n\]+\t.\[^\t \]+\t0x2\[\t \]+# cta_index\[\\r\\n\]+\t.\[^\t \]+\t0x3\[\t \]+# cta_nelems" 1 } } */
> +/* { dg-final { scan-assembler-times "\t.\[^\t \]+\t0x4\[\t \]+# cta_contents\[\\r\\n\]+\t.\[^\t \]+\t0x2\[\t \]+# cta_index\[\\r\\n\]+\t.\[^\t \]+\t0x2\[\t \]+# cta_nelems" 1 } } */
> +
> +int a[2][3][4];

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH, V3] ctf: fix incorrect CTF for multi-dimensional array types
  2024-03-04 18:00 ` [PATCH,V2] ctf: fix incorrect CTF for multi-dimensional array types Indu Bhagat
  2024-03-04 19:46   ` David Faust
@ 2024-03-05  8:47   ` Indu Bhagat
  2024-03-05 17:41     ` David Faust
  1 sibling, 1 reply; 5+ messages in thread
From: Indu Bhagat @ 2024-03-05  8:47 UTC (permalink / raw)
  To: gcc-patches; +Cc: david.faust, jose.marchesi, cupertino.miranda, Indu Bhagat

From: Cupertino Miranda <cupertino.miranda@oracle.com>

[Changes from V2]
  - Fixed aarch64 new FAILs reported by Linaro CI.
  - Fixed typos and other nits pointed out in V2.
[End of changes from V2]

PR debug/114186

DWARF DIEs of type DW_TAG_subrange_type are linked together to represent
the information about the subsequent dimensions.  The CTF processing was
so far working through them in the opposite (incorrect) order.

While fixing the issue, refactor the code a bit for readability.

co-authored-By: Indu Bhagat <indu.bhagat@oracle.com>

gcc/
	PR debug/114186
	* dwarf2ctf.cc (gen_ctf_array_type): Invoke the ctf_add_array ()
	in the correct order of the dimensions.
        (gen_ctf_subrange_type): Refactor out handling of
	DW_TAG_subrange_type DIE to here.

gcc/testsuite/
	PR debug/114186
	* gcc.dg/debug/ctf/ctf-array-6.c: Add test.
---

Testing notes:
 - Linaro CI reported three new FAILs introduced by ctf-array-6.c due to
   presence of char '#' on aarch64 where the ASM_COMMENT_START differs.
   Fixed and regression tested on aarch64.
 - Regression tested on x86_64-linux-gnu default target.
 - Regression tested for target bpf-unknown-none (btf.exp, ctf.exp, bpf.exp).
 - Kernel build with -gctf shows healthier CTF types for arrays.

---
 gcc/dwarf2ctf.cc                             | 158 +++++++++----------
 gcc/testsuite/gcc.dg/debug/ctf/ctf-array-6.c |  14 ++
 2 files changed, 89 insertions(+), 83 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/debug/ctf/ctf-array-6.c

diff --git a/gcc/dwarf2ctf.cc b/gcc/dwarf2ctf.cc
index dca86edfffa9..77d6bf896893 100644
--- a/gcc/dwarf2ctf.cc
+++ b/gcc/dwarf2ctf.cc
@@ -349,105 +349,97 @@ gen_ctf_pointer_type (ctf_container_ref ctfc, dw_die_ref ptr_type)
   return ptr_type_id;
 }
 
-/* Generate CTF for an array type.  */
+/* Recursively generate CTF for array dimensions starting at DIE C (of type
+   DW_TAG_subrange_type) until DIE LAST (of type DW_TAG_subrange_type) is
+   reached.  ARRAY_ELEMS_TYPE_ID is base type for the array.  */
 
 static ctf_id_t
-gen_ctf_array_type (ctf_container_ref ctfc, dw_die_ref array_type)
+gen_ctf_subrange_type (ctf_container_ref ctfc, ctf_id_t array_elems_type_id,
+		       dw_die_ref c, dw_die_ref last)
 {
-  dw_die_ref c;
-  ctf_id_t array_elems_type_id = CTF_NULL_TYPEID;
+  ctf_arinfo_t arinfo;
+  ctf_id_t array_node_type_id = CTF_NULL_TYPEID;
 
-  int vector_type_p = get_AT_flag (array_type, DW_AT_GNU_vector);
-  if (vector_type_p)
-    return array_elems_type_id;
+  dw_attr_node *upper_bound_at;
+  dw_die_ref array_index_type;
+  uint32_t array_num_elements;
 
-  dw_die_ref array_elems_type = ctf_get_AT_type (array_type);
+  if (dw_get_die_tag (c) == DW_TAG_subrange_type)
+    {
+      /* When DW_AT_upper_bound is used to specify the size of an
+	 array in DWARF, it is usually an unsigned constant
+	 specifying the upper bound index of the array.  However,
+	 for unsized arrays, such as foo[] or bar[0],
+	 DW_AT_upper_bound is a signed integer constant
+	 instead.  */
+
+      upper_bound_at = get_AT (c, DW_AT_upper_bound);
+      if (upper_bound_at
+	  && AT_class (upper_bound_at) == dw_val_class_unsigned_const)
+	/* This is the upper bound index.  */
+	array_num_elements = get_AT_unsigned (c, DW_AT_upper_bound) + 1;
+      else if (get_AT (c, DW_AT_count))
+	array_num_elements = get_AT_unsigned (c, DW_AT_count);
+      else
+	{
+	  /* This is a VLA of some kind.  */
+	  array_num_elements = 0;
+	}
+    }
+  else
+    gcc_unreachable ();
 
-  /* First, register the type of the array elements if needed.  */
-  array_elems_type_id = gen_ctf_type (ctfc, array_elems_type);
+  /* Ok, mount and register the array type.  Note how the array
+     type we register here is the type of the elements in
+     subsequent "dimensions", if there are any.  */
+  arinfo.ctr_nelems = array_num_elements;
 
-  /* DWARF array types pretend C supports multi-dimensional arrays.
-     So for the type int[N][M], the array type DIE contains two
-     subrange_type children, the first with upper bound N-1 and the
-     second with upper bound M-1.
+  array_index_type = ctf_get_AT_type (c);
+  arinfo.ctr_index = gen_ctf_type (ctfc, array_index_type);
 
-     CTF, on the other hand, just encodes each array type in its own
-     array type CTF struct.  Therefore we have to iterate on the
-     children and create all the needed types.  */
+  if (c == last)
+    arinfo.ctr_contents = array_elems_type_id;
+  else
+    arinfo.ctr_contents = gen_ctf_subrange_type (ctfc, array_elems_type_id,
+						 dw_get_die_sib (c), last);
 
-  c = dw_get_die_child (array_type);
-  gcc_assert (c);
-  do
-    {
-      ctf_arinfo_t arinfo;
-      dw_die_ref array_index_type;
-      uint32_t array_num_elements;
+  if (!ctf_type_exists (ctfc, c, &array_node_type_id))
+    array_node_type_id = ctf_add_array (ctfc, CTF_ADD_ROOT, &arinfo, c);
 
-      c = dw_get_die_sib (c);
+  return array_node_type_id;
+}
 
-      if (dw_get_die_tag (c) == DW_TAG_subrange_type)
-	{
-	  dw_attr_node *upper_bound_at;
-
-	  array_index_type = ctf_get_AT_type (c);
-
-	  /* When DW_AT_upper_bound is used to specify the size of an
-	     array in DWARF, it is usually an unsigned constant
-	     specifying the upper bound index of the array.  However,
-	     for unsized arrays, such as foo[] or bar[0],
-	     DW_AT_upper_bound is a signed integer constant
-	     instead.  */
-
-	  upper_bound_at = get_AT (c, DW_AT_upper_bound);
-	  if (upper_bound_at
-	      && AT_class (upper_bound_at) == dw_val_class_unsigned_const)
-	    /* This is the upper bound index.  */
-	    array_num_elements = get_AT_unsigned (c, DW_AT_upper_bound) + 1;
-	  else if (get_AT (c, DW_AT_count))
-	    array_num_elements = get_AT_unsigned (c, DW_AT_count);
-	  else
-	    {
-	      /* This is a VLA of some kind.  */
-	      array_num_elements = 0;
-	    }
-	}
-      else if (dw_get_die_tag (c) == DW_TAG_enumeration_type)
-	{
-	  array_index_type = 0;
-	  array_num_elements = 0;
-	  /* XXX writeme. */
-	  gcc_assert (1);
-	}
-      else
-	gcc_unreachable ();
+/* Generate CTF for an ARRAY_TYPE.  */
 
-      /* Ok, mount and register the array type.  Note how the array
-	 type we register here is the type of the elements in
-	 subsequent "dimensions", if there are any.  */
+static ctf_id_t
+gen_ctf_array_type (ctf_container_ref ctfc,
+		    dw_die_ref array_type)
+{
+  dw_die_ref first, last, array_elems_type;
+  ctf_id_t array_elems_type_id = CTF_NULL_TYPEID;
+  ctf_id_t array_type_id = CTF_NULL_TYPEID;
 
-      arinfo.ctr_nelems = array_num_elements;
-      if (array_index_type)
-	arinfo.ctr_index = gen_ctf_type (ctfc, array_index_type);
-      else
-	arinfo.ctr_index = gen_ctf_type (ctfc, ctf_array_index_die);
+  int vector_type_p = get_AT_flag (array_type, DW_AT_GNU_vector);
+  if (vector_type_p)
+    return array_elems_type_id;
 
-      arinfo.ctr_contents = array_elems_type_id;
-      if (!ctf_type_exists (ctfc, c, &array_elems_type_id))
-	array_elems_type_id = ctf_add_array (ctfc, CTF_ADD_ROOT, &arinfo,
-					     c);
-    }
-  while (c != dw_get_die_child (array_type));
+  /* Find the first and last array dimension DIEs.  */
+  last = dw_get_die_child (array_type);
+  first = dw_get_die_sib (last);
 
-#if 0
   /* Type de-duplication.
-     Consult the ctfc_types hash again before adding the CTF array type because
-     there can be cases where an array_type type may have been added by the
-     gen_ctf_type call above.  */
-  if (!ctf_type_exists (ctfc, array_type, &type_id))
-    type_id = ctf_add_array (ctfc, CTF_ADD_ROOT, &arinfo, array_type);
-#endif
-
-  return array_elems_type_id;
+     Consult the ctfc_types before adding CTF type for the first dimension.  */
+  if (!ctf_type_exists (ctfc, first, &array_type_id))
+    {
+      array_elems_type = ctf_get_AT_type (array_type);
+      /* First, register the type of the array elements if needed.  */
+      array_elems_type_id = gen_ctf_type (ctfc, array_elems_type);
+
+      array_type_id = gen_ctf_subrange_type (ctfc, array_elems_type_id, first,
+					     last);
+    }
+
+  return array_type_id;
 }
 
 /* Generate CTF for a typedef.  */
diff --git a/gcc/testsuite/gcc.dg/debug/ctf/ctf-array-6.c b/gcc/testsuite/gcc.dg/debug/ctf/ctf-array-6.c
new file mode 100644
index 000000000000..5564cb8865db
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/debug/ctf/ctf-array-6.c
@@ -0,0 +1,14 @@
+/* CTF generation for multidimensional array.  */
+
+/* { dg-do compile )  */
+/* { dg-options "-O0 -gctf -dA" } */
+
+/* { dg-final { scan-assembler-times "\[\t \]+0x2\[\t \]+\[^\n\]*cta_nelems" 1 } } */
+/* { dg-final { scan-assembler-times "\[\t \]+0x3\[\t \]+\[^\n\]*cta_nelems" 1 } } */
+/* { dg-final { scan-assembler-times "\[\t \]+0x4\[\t \]+\[^\n\]*cta_nelems" 1 } } */
+
+/* { dg-final { scan-assembler-times "\[\t \]+0x1\[\t \]+\[^\n\]*cta_contents\[\\r\\n\]+\[^\\r\\n\]*\[\\r\\n\]+\[^\\r\\n\]*0x4\[\t \]+\[^\n\]*cta_nelems" 1 } } */
+/* { dg-final { scan-assembler-times "\[\t \]+0x3\[\t \]+\[^\n\]*cta_contents\[\\r\\n\]+\[^\\r\\n\]*\[\\r\\n\]+\[^\\r\\n\]*0x3\[\t \]+\[^\n\]*cta_nelems" 1 } } */
+/* { dg-final { scan-assembler-times "\[\t \]+0x4\[\t \]+\[^\n\]*cta_contents\[\\r\\n\]+\[^\\r\\n\]*\[\\r\\n\]+\[^\\r\\n\]*0x2\[\t \]+\[^\n\]*cta_nelems" 1 } } */
+
+int a[2][3][4];
-- 
2.43.0


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH, V3] ctf: fix incorrect CTF for multi-dimensional array types
  2024-03-05  8:47   ` [PATCH, V3] " Indu Bhagat
@ 2024-03-05 17:41     ` David Faust
  0 siblings, 0 replies; 5+ messages in thread
From: David Faust @ 2024-03-05 17:41 UTC (permalink / raw)
  To: Indu Bhagat, cupertino.miranda; +Cc: gcc-patches, jose.marchesi



On 3/5/24 00:47, Indu Bhagat wrote:
> From: Cupertino Miranda <cupertino.miranda@oracle.com>
> 
> [Changes from V2]
>   - Fixed aarch64 new FAILs reported by Linaro CI.
>   - Fixed typos and other nits pointed out in V2.
> [End of changes from V2]

OK, thanks.

> 
> PR debug/114186
> 
> DWARF DIEs of type DW_TAG_subrange_type are linked together to represent
> the information about the subsequent dimensions.  The CTF processing was
> so far working through them in the opposite (incorrect) order.
> 
> While fixing the issue, refactor the code a bit for readability.
> 
> co-authored-By: Indu Bhagat <indu.bhagat@oracle.com>
> 
> gcc/
> 	PR debug/114186
> 	* dwarf2ctf.cc (gen_ctf_array_type): Invoke the ctf_add_array ()
> 	in the correct order of the dimensions.
>         (gen_ctf_subrange_type): Refactor out handling of
> 	DW_TAG_subrange_type DIE to here.
> 
> gcc/testsuite/
> 	PR debug/114186
> 	* gcc.dg/debug/ctf/ctf-array-6.c: Add test.
> ---
> 
> Testing notes:
>  - Linaro CI reported three new FAILs introduced by ctf-array-6.c due to
>    presence of char '#' on aarch64 where the ASM_COMMENT_START differs.
>    Fixed and regression tested on aarch64.
>  - Regression tested on x86_64-linux-gnu default target.
>  - Regression tested for target bpf-unknown-none (btf.exp, ctf.exp, bpf.exp).
>  - Kernel build with -gctf shows healthier CTF types for arrays.
> 
> ---
>  gcc/dwarf2ctf.cc                             | 158 +++++++++----------
>  gcc/testsuite/gcc.dg/debug/ctf/ctf-array-6.c |  14 ++
>  2 files changed, 89 insertions(+), 83 deletions(-)
>  create mode 100644 gcc/testsuite/gcc.dg/debug/ctf/ctf-array-6.c
> 
> diff --git a/gcc/dwarf2ctf.cc b/gcc/dwarf2ctf.cc
> index dca86edfffa9..77d6bf896893 100644
> --- a/gcc/dwarf2ctf.cc
> +++ b/gcc/dwarf2ctf.cc
> @@ -349,105 +349,97 @@ gen_ctf_pointer_type (ctf_container_ref ctfc, dw_die_ref ptr_type)
>    return ptr_type_id;
>  }
>  
> -/* Generate CTF for an array type.  */
> +/* Recursively generate CTF for array dimensions starting at DIE C (of type
> +   DW_TAG_subrange_type) until DIE LAST (of type DW_TAG_subrange_type) is
> +   reached.  ARRAY_ELEMS_TYPE_ID is base type for the array.  */
>  
>  static ctf_id_t
> -gen_ctf_array_type (ctf_container_ref ctfc, dw_die_ref array_type)
> +gen_ctf_subrange_type (ctf_container_ref ctfc, ctf_id_t array_elems_type_id,
> +		       dw_die_ref c, dw_die_ref last)
>  {
> -  dw_die_ref c;
> -  ctf_id_t array_elems_type_id = CTF_NULL_TYPEID;
> +  ctf_arinfo_t arinfo;
> +  ctf_id_t array_node_type_id = CTF_NULL_TYPEID;
>  
> -  int vector_type_p = get_AT_flag (array_type, DW_AT_GNU_vector);
> -  if (vector_type_p)
> -    return array_elems_type_id;
> +  dw_attr_node *upper_bound_at;
> +  dw_die_ref array_index_type;
> +  uint32_t array_num_elements;
>  
> -  dw_die_ref array_elems_type = ctf_get_AT_type (array_type);
> +  if (dw_get_die_tag (c) == DW_TAG_subrange_type)
> +    {
> +      /* When DW_AT_upper_bound is used to specify the size of an
> +	 array in DWARF, it is usually an unsigned constant
> +	 specifying the upper bound index of the array.  However,
> +	 for unsized arrays, such as foo[] or bar[0],
> +	 DW_AT_upper_bound is a signed integer constant
> +	 instead.  */
> +
> +      upper_bound_at = get_AT (c, DW_AT_upper_bound);
> +      if (upper_bound_at
> +	  && AT_class (upper_bound_at) == dw_val_class_unsigned_const)
> +	/* This is the upper bound index.  */
> +	array_num_elements = get_AT_unsigned (c, DW_AT_upper_bound) + 1;
> +      else if (get_AT (c, DW_AT_count))
> +	array_num_elements = get_AT_unsigned (c, DW_AT_count);
> +      else
> +	{
> +	  /* This is a VLA of some kind.  */
> +	  array_num_elements = 0;
> +	}
> +    }
> +  else
> +    gcc_unreachable ();
>  
> -  /* First, register the type of the array elements if needed.  */
> -  array_elems_type_id = gen_ctf_type (ctfc, array_elems_type);
> +  /* Ok, mount and register the array type.  Note how the array
> +     type we register here is the type of the elements in
> +     subsequent "dimensions", if there are any.  */
> +  arinfo.ctr_nelems = array_num_elements;
>  
> -  /* DWARF array types pretend C supports multi-dimensional arrays.
> -     So for the type int[N][M], the array type DIE contains two
> -     subrange_type children, the first with upper bound N-1 and the
> -     second with upper bound M-1.
> +  array_index_type = ctf_get_AT_type (c);
> +  arinfo.ctr_index = gen_ctf_type (ctfc, array_index_type);
>  
> -     CTF, on the other hand, just encodes each array type in its own
> -     array type CTF struct.  Therefore we have to iterate on the
> -     children and create all the needed types.  */
> +  if (c == last)
> +    arinfo.ctr_contents = array_elems_type_id;
> +  else
> +    arinfo.ctr_contents = gen_ctf_subrange_type (ctfc, array_elems_type_id,
> +						 dw_get_die_sib (c), last);
>  
> -  c = dw_get_die_child (array_type);
> -  gcc_assert (c);
> -  do
> -    {
> -      ctf_arinfo_t arinfo;
> -      dw_die_ref array_index_type;
> -      uint32_t array_num_elements;
> +  if (!ctf_type_exists (ctfc, c, &array_node_type_id))
> +    array_node_type_id = ctf_add_array (ctfc, CTF_ADD_ROOT, &arinfo, c);
>  
> -      c = dw_get_die_sib (c);
> +  return array_node_type_id;
> +}
>  
> -      if (dw_get_die_tag (c) == DW_TAG_subrange_type)
> -	{
> -	  dw_attr_node *upper_bound_at;
> -
> -	  array_index_type = ctf_get_AT_type (c);
> -
> -	  /* When DW_AT_upper_bound is used to specify the size of an
> -	     array in DWARF, it is usually an unsigned constant
> -	     specifying the upper bound index of the array.  However,
> -	     for unsized arrays, such as foo[] or bar[0],
> -	     DW_AT_upper_bound is a signed integer constant
> -	     instead.  */
> -
> -	  upper_bound_at = get_AT (c, DW_AT_upper_bound);
> -	  if (upper_bound_at
> -	      && AT_class (upper_bound_at) == dw_val_class_unsigned_const)
> -	    /* This is the upper bound index.  */
> -	    array_num_elements = get_AT_unsigned (c, DW_AT_upper_bound) + 1;
> -	  else if (get_AT (c, DW_AT_count))
> -	    array_num_elements = get_AT_unsigned (c, DW_AT_count);
> -	  else
> -	    {
> -	      /* This is a VLA of some kind.  */
> -	      array_num_elements = 0;
> -	    }
> -	}
> -      else if (dw_get_die_tag (c) == DW_TAG_enumeration_type)
> -	{
> -	  array_index_type = 0;
> -	  array_num_elements = 0;
> -	  /* XXX writeme. */
> -	  gcc_assert (1);
> -	}
> -      else
> -	gcc_unreachable ();
> +/* Generate CTF for an ARRAY_TYPE.  */
>  
> -      /* Ok, mount and register the array type.  Note how the array
> -	 type we register here is the type of the elements in
> -	 subsequent "dimensions", if there are any.  */
> +static ctf_id_t
> +gen_ctf_array_type (ctf_container_ref ctfc,
> +		    dw_die_ref array_type)
> +{
> +  dw_die_ref first, last, array_elems_type;
> +  ctf_id_t array_elems_type_id = CTF_NULL_TYPEID;
> +  ctf_id_t array_type_id = CTF_NULL_TYPEID;
>  
> -      arinfo.ctr_nelems = array_num_elements;
> -      if (array_index_type)
> -	arinfo.ctr_index = gen_ctf_type (ctfc, array_index_type);
> -      else
> -	arinfo.ctr_index = gen_ctf_type (ctfc, ctf_array_index_die);
> +  int vector_type_p = get_AT_flag (array_type, DW_AT_GNU_vector);
> +  if (vector_type_p)
> +    return array_elems_type_id;
>  
> -      arinfo.ctr_contents = array_elems_type_id;
> -      if (!ctf_type_exists (ctfc, c, &array_elems_type_id))
> -	array_elems_type_id = ctf_add_array (ctfc, CTF_ADD_ROOT, &arinfo,
> -					     c);
> -    }
> -  while (c != dw_get_die_child (array_type));
> +  /* Find the first and last array dimension DIEs.  */
> +  last = dw_get_die_child (array_type);
> +  first = dw_get_die_sib (last);
>  
> -#if 0
>    /* Type de-duplication.
> -     Consult the ctfc_types hash again before adding the CTF array type because
> -     there can be cases where an array_type type may have been added by the
> -     gen_ctf_type call above.  */
> -  if (!ctf_type_exists (ctfc, array_type, &type_id))
> -    type_id = ctf_add_array (ctfc, CTF_ADD_ROOT, &arinfo, array_type);
> -#endif
> -
> -  return array_elems_type_id;
> +     Consult the ctfc_types before adding CTF type for the first dimension.  */
> +  if (!ctf_type_exists (ctfc, first, &array_type_id))
> +    {
> +      array_elems_type = ctf_get_AT_type (array_type);
> +      /* First, register the type of the array elements if needed.  */
> +      array_elems_type_id = gen_ctf_type (ctfc, array_elems_type);
> +
> +      array_type_id = gen_ctf_subrange_type (ctfc, array_elems_type_id, first,
> +					     last);
> +    }
> +
> +  return array_type_id;
>  }
>  
>  /* Generate CTF for a typedef.  */
> diff --git a/gcc/testsuite/gcc.dg/debug/ctf/ctf-array-6.c b/gcc/testsuite/gcc.dg/debug/ctf/ctf-array-6.c
> new file mode 100644
> index 000000000000..5564cb8865db
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/debug/ctf/ctf-array-6.c
> @@ -0,0 +1,14 @@
> +/* CTF generation for multidimensional array.  */
> +
> +/* { dg-do compile )  */
> +/* { dg-options "-O0 -gctf -dA" } */
> +
> +/* { dg-final { scan-assembler-times "\[\t \]+0x2\[\t \]+\[^\n\]*cta_nelems" 1 } } */
> +/* { dg-final { scan-assembler-times "\[\t \]+0x3\[\t \]+\[^\n\]*cta_nelems" 1 } } */
> +/* { dg-final { scan-assembler-times "\[\t \]+0x4\[\t \]+\[^\n\]*cta_nelems" 1 } } */
> +
> +/* { dg-final { scan-assembler-times "\[\t \]+0x1\[\t \]+\[^\n\]*cta_contents\[\\r\\n\]+\[^\\r\\n\]*\[\\r\\n\]+\[^\\r\\n\]*0x4\[\t \]+\[^\n\]*cta_nelems" 1 } } */
> +/* { dg-final { scan-assembler-times "\[\t \]+0x3\[\t \]+\[^\n\]*cta_contents\[\\r\\n\]+\[^\\r\\n\]*\[\\r\\n\]+\[^\\r\\n\]*0x3\[\t \]+\[^\n\]*cta_nelems" 1 } } */
> +/* { dg-final { scan-assembler-times "\[\t \]+0x4\[\t \]+\[^\n\]*cta_contents\[\\r\\n\]+\[^\\r\\n\]*\[\\r\\n\]+\[^\\r\\n\]*0x2\[\t \]+\[^\n\]*cta_nelems" 1 } } */
> +
> +int a[2][3][4];

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2024-03-05 17:41 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-02-28 15:03 [PATCH] ctf: Fix multi-dimentional array types ordering in CTF Cupertino Miranda
2024-03-04 18:00 ` [PATCH,V2] ctf: fix incorrect CTF for multi-dimensional array types Indu Bhagat
2024-03-04 19:46   ` David Faust
2024-03-05  8:47   ` [PATCH, V3] " Indu Bhagat
2024-03-05 17:41     ` David Faust

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).