public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] btf: Add support to BTF_KIND_ENUM64 type
@ 2022-08-29 21:11 Guillermo E. Martinez
  2022-09-20 19:32 ` Guillermo E. Martinez
                   ` (2 more replies)
  0 siblings, 3 replies; 15+ messages in thread
From: Guillermo E. Martinez @ 2022-08-29 21:11 UTC (permalink / raw)
  To: gcc-patches; +Cc: Guillermo E. Martinez

Hello GCC team,

The following patch update BTF/CTF backend to support
BTF_KIND_ENUM64 type.

Comments will be welcomed and appreciated!,

Kind regards,
guillermo
--

BTF supports 64-bits enumerators with following encoding:

  struct btf_type:
    name_off: 0 or offset to a valid C identifier
    info.kind_flag: 0 for unsigned, 1 for signed
    info.kind: BTF_KIND_ENUM64
    info.vlen: number of enum values
    size: 1/2/4/8

The btf_type is followed by info.vlen number of:

    struct btf_enum64
    {
      uint32_t name_off;   /* Offset in string section of enumerator name.  */
      uint32_t val_lo32;   /* lower 32-bit value for a 64-bit value Enumerator */
      uint32_t val_hi32;   /* high 32-bit value for a 64-bit value Enumerator */
    };

So, a new btf_enum64 structure was added to represent BTF_KIND_ENUM64
and a new field in ctf_dtdef to represent specific type's properties, in
the particular case for CTF enums it helps to distinguish when its
enumerators values are signed or unsigned, later that information is
used to encode the BTF enum type.

gcc/ChangeLog:

	* btfout.cc (btf_calc_num_vbytes): Compute enumeration size depending of
	enumerator type btf_enum{,64}.
	(btf_asm_type): Update btf_kflag according to enumerators sign,
	using correct BPF type in BTF_KIND_ENUMi{,64}.
	(btf_asm_enum_const): New argument to represent the size of
	the BTF enum type.
	* ctfc.cc (ctf_add_enum): Use and initialization of flag field to
	CTF_ENUM_F_NONE.
	(ctf_add_enumerator): New argument to represent CTF flags,
	updating the comment and flag vaue according to enumerators
	sing.
	* ctfc.h (ctf_dmdef): Update dmd_value to HOST_WIDE_INT to allow
	use 32/64 bits enumerators.
	(ctf_dtdef): Add flags to to describe specifyc type's properties.
	* dwarf2ctf.cc (gen_ctf_enumeration_type): Update flags field
	depending when a signed enumerator value is found.
	include/btf.h (btf_enum64): Add new definition and new symbolic
	constant to BTF_KIND_ENUM64 and BTF_KF_ENUM_{UN,}SIGNED.

gcc/testsuite/ChangeLog:

	gcc.dg/debug/btf/btf-enum-1.c: Update testcase, with correct
	info.kflags encoding.
	gcc.dg/debug/btf/btf-enum64-1.c: New testcase.
---
 gcc/btfout.cc                                 | 24 ++++++++---
 gcc/ctfc.cc                                   | 14 ++++---
 gcc/ctfc.h                                    |  9 +++-
 gcc/dwarf2ctf.cc                              |  9 +++-
 gcc/testsuite/gcc.dg/debug/btf/btf-enum-1.c   |  2 +-
 gcc/testsuite/gcc.dg/debug/btf/btf-enum64-1.c | 41 +++++++++++++++++++
 include/btf.h                                 | 19 +++++++--
 7 files changed, 99 insertions(+), 19 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/debug/btf/btf-enum64-1.c

diff --git a/gcc/btfout.cc b/gcc/btfout.cc
index 997a33fa089..4b11c867c23 100644
--- a/gcc/btfout.cc
+++ b/gcc/btfout.cc
@@ -223,7 +223,9 @@ btf_calc_num_vbytes (ctf_dtdef_ref dtd)
       break;
 
     case BTF_KIND_ENUM:
-      vlen_bytes += vlen * sizeof (struct btf_enum);
+      vlen_bytes += (dtd->dtd_data.ctti_size == 0x8)
+			? vlen * sizeof (struct btf_enum64)
+			: vlen * sizeof (struct btf_enum);
       break;
 
     case BTF_KIND_FUNC_PROTO:
@@ -622,6 +624,15 @@ btf_asm_type (ctf_container_ref ctfc, ctf_dtdef_ref dtd)
       btf_size_type = 0;
     }
 
+ if (btf_kind == BTF_KIND_ENUM)
+   {
+     btf_kflag = (dtd->flags & CTF_ENUM_F_ENUMERATORS_SIGNED)
+		    ? BTF_KF_ENUM_SIGNED
+		    : BTF_KF_ENUM_UNSIGNED;
+     if (dtd->dtd_data.ctti_size == 0x8)
+       btf_kind = BTF_KIND_ENUM64;
+   }
+
   dw2_asm_output_data (4, dtd->dtd_data.ctti_name, "btt_name");
   dw2_asm_output_data (4, BTF_TYPE_INFO (btf_kind, btf_kflag, btf_vlen),
 		       "btt_info: kind=%u, kflag=%u, vlen=%u",
@@ -634,6 +645,7 @@ btf_asm_type (ctf_container_ref ctfc, ctf_dtdef_ref dtd)
     case BTF_KIND_UNION:
     case BTF_KIND_ENUM:
     case BTF_KIND_DATASEC:
+    case BTF_KIND_ENUM64:
       dw2_asm_output_data (4, dtd->dtd_data.ctti_size, "btt_size: %uB",
 			   dtd->dtd_data.ctti_size);
       return;
@@ -707,13 +719,13 @@ btf_asm_sou_member (ctf_container_ref ctfc, ctf_dmdef_t * dmd)
     }
 }
 
-/* Asm'out an enum constant following a BTF_KIND_ENUM.  */
+/* Asm'out an enum constant following a BTF_KIND_ENUM{,64}.  */
 
 static void
-btf_asm_enum_const (ctf_dmdef_t * dmd)
+btf_asm_enum_const (unsigned int size, ctf_dmdef_t * dmd)
 {
   dw2_asm_output_data (4, dmd->dmd_name_offset, "bte_name");
-  dw2_asm_output_data (4, dmd->dmd_value, "bte_value");
+  dw2_asm_output_data (size, dmd->dmd_value, "bte_value");
 }
 
 /* Asm'out a function parameter description following a BTF_KIND_FUNC_PROTO.  */
@@ -871,7 +883,7 @@ output_asm_btf_sou_fields (ctf_container_ref ctfc, ctf_dtdef_ref dtd)
       btf_asm_sou_member (ctfc, dmd);
 }
 
-/* Output all enumerator constants following a BTF_KIND_ENUM.  */
+/* Output all enumerator constants following a BTF_KIND_ENUM{,64}.  */
 
 static void
 output_asm_btf_enum_list (ctf_container_ref ARG_UNUSED (ctfc),
@@ -881,7 +893,7 @@ output_asm_btf_enum_list (ctf_container_ref ARG_UNUSED (ctfc),
 
   for (dmd = dtd->dtd_u.dtu_members;
        dmd != NULL; dmd = (ctf_dmdef_t *) ctf_dmd_list_next (dmd))
-    btf_asm_enum_const (dmd);
+    btf_asm_enum_const (dtd->dtd_data.ctti_size, dmd);
 }
 
 /* Output all function arguments following a BTF_KIND_FUNC_PROTO.  */
diff --git a/gcc/ctfc.cc b/gcc/ctfc.cc
index 9773358a475..253c36b6a0a 100644
--- a/gcc/ctfc.cc
+++ b/gcc/ctfc.cc
@@ -604,6 +604,7 @@ ctf_add_enum (ctf_container_ref ctfc, uint32_t flag, const char * name,
   gcc_assert (size <= CTF_MAX_SIZE);
 
   dtd->dtd_data.ctti_size = size;
+  dtd->flags = CTF_ENUM_F_NONE;
 
   ctfc->ctfc_num_stypes++;
 
@@ -612,7 +613,7 @@ ctf_add_enum (ctf_container_ref ctfc, uint32_t flag, const char * name,
 
 int
 ctf_add_enumerator (ctf_container_ref ctfc, ctf_id_t enid, const char * name,
-		    HOST_WIDE_INT value, dw_die_ref die)
+		    HOST_WIDE_INT value, uint32_t flags, dw_die_ref die)
 {
   ctf_dmdef_t * dmd;
   uint32_t kind, vlen, root;
@@ -630,10 +631,12 @@ ctf_add_enumerator (ctf_container_ref ctfc, ctf_id_t enid, const char * name,
 
   gcc_assert (kind == CTF_K_ENUM && vlen < CTF_MAX_VLEN);
 
-  /* Enum value is of type HOST_WIDE_INT in the compiler, dmd_value is int32_t
-     on the other hand.  Check bounds and skip adding this enum value if out of
-     bounds.  */
-  if ((value > INT_MAX) || (value < INT_MIN))
+  /* Enum value is of type HOST_WIDE_INT in the compiler, CTF enumerators
+     values in ctf_enum_t is limited to int32_t, BTF supports signed and
+     unsigned enumerators values of 32 and 64 bits, for both debug formats
+     we use ctf_dmdef_t.dmd_value entry of HOST_WIDE_INT type. So check
+     CTF bounds and skip adding this enum value if out of bounds.  */
+  if (!btf_debuginfo_p() && ((value > INT_MAX) || (value < INT_MIN)))
     {
       /* FIXME - Note this TBD_CTF_REPRESENTATION_LIMIT.  */
       return (1);
@@ -649,6 +652,7 @@ ctf_add_enumerator (ctf_container_ref ctfc, ctf_id_t enid, const char * name,
   dmd->dmd_value = value;
 
   dtd->dtd_data.ctti_info = CTF_TYPE_INFO (kind, root, vlen + 1);
+  dtd->flags |= flags;
   ctf_dmd_list_append (&dtd->dtd_u.dtu_members, dmd);
 
   if ((name != NULL) && strcmp (name, ""))
diff --git a/gcc/ctfc.h b/gcc/ctfc.h
index bcf3a43ae1b..a22342b2610 100644
--- a/gcc/ctfc.h
+++ b/gcc/ctfc.h
@@ -125,6 +125,10 @@ typedef struct GTY (()) ctf_itype
 
 #define CTF_FUNC_VARARG 0x1
 
+/* Enum specific flags.  */
+#define CTF_ENUM_F_NONE                     (0)
+#define CTF_ENUM_F_ENUMERATORS_SIGNED       (1 << 0)
+
 /* Struct/union/enum member definition for CTF generation.  */
 
 typedef struct GTY ((chain_next ("%h.dmd_next"))) ctf_dmdef
@@ -133,7 +137,7 @@ typedef struct GTY ((chain_next ("%h.dmd_next"))) ctf_dmdef
   ctf_id_t dmd_type;		/* Type of this member (for sou).  */
   uint32_t dmd_name_offset;	/* Offset of the name in str table.  */
   uint64_t dmd_offset;		/* Offset of this member in bits (for sou).  */
-  int dmd_value;		/* Value of this member (for enum).  */
+  HOST_WIDE_INT dmd_value;	/* Value of this member (for enum).  */
   struct ctf_dmdef * dmd_next;	/* A list node.  */
 } ctf_dmdef_t;
 
@@ -162,6 +166,7 @@ struct GTY ((for_user)) ctf_dtdef
   bool from_global_func; /* Whether this type was added from a global
 			    function.  */
   uint32_t linkage;           /* Used in function types.  0=local, 1=global.  */
+  uint32_t flags;             /* Flags to describe specific type's properties.  */
   union GTY ((desc ("ctf_dtu_d_union_selector (&%1)")))
   {
     /* struct, union, or enum.  */
@@ -429,7 +434,7 @@ extern ctf_id_t ctf_add_sou (ctf_container_ref, uint32_t, const char *,
 			     uint32_t, size_t, dw_die_ref);
 
 extern int ctf_add_enumerator (ctf_container_ref, ctf_id_t, const char *,
-			       HOST_WIDE_INT, dw_die_ref);
+			       HOST_WIDE_INT, uint32_t, dw_die_ref);
 extern int ctf_add_member_offset (ctf_container_ref, dw_die_ref, const char *,
 				  ctf_id_t, uint64_t);
 extern int ctf_add_function_arg (ctf_container_ref, dw_die_ref,
diff --git a/gcc/dwarf2ctf.cc b/gcc/dwarf2ctf.cc
index 397100004c2..0ef96dd48fd 100644
--- a/gcc/dwarf2ctf.cc
+++ b/gcc/dwarf2ctf.cc
@@ -772,6 +772,7 @@ gen_ctf_enumeration_type (ctf_container_ref ctfc, dw_die_ref enumeration)
 	  const char *enumerator_name;
 	  dw_attr_node *enumerator_value;
 	  HOST_WIDE_INT value_wide_int;
+	  uint32_t flags = 0;
 
 	  c = dw_get_die_sib (c);
 
@@ -785,10 +786,14 @@ gen_ctf_enumeration_type (ctf_container_ref ctfc, dw_die_ref enumeration)
 		  == dw_val_class_unsigned_const_implicit))
 	    value_wide_int = AT_unsigned (enumerator_value);
 	  else
-	    value_wide_int = AT_int (enumerator_value);
+	    {
+	      value_wide_int = AT_int (enumerator_value);
+	      flags |= CTF_ENUM_F_ENUMERATORS_SIGNED;
+	    }
 
 	  ctf_add_enumerator (ctfc, enumeration_type_id,
-			      enumerator_name, value_wide_int, enumeration);
+			      enumerator_name, value_wide_int,
+			      flags, enumeration);
 	}
       while (c != dw_get_die_child (enumeration));
   }
diff --git a/gcc/testsuite/gcc.dg/debug/btf/btf-enum-1.c b/gcc/testsuite/gcc.dg/debug/btf/btf-enum-1.c
index 728493b0804..7e940529f1b 100644
--- a/gcc/testsuite/gcc.dg/debug/btf/btf-enum-1.c
+++ b/gcc/testsuite/gcc.dg/debug/btf/btf-enum-1.c
@@ -4,7 +4,7 @@
 /* { dg-options "-O0 -gbtf -dA" } */
 
 /* { dg-final { scan-assembler-times "\[\t \]0x6000004\[\t \]+\[^\n\]*btt_info" 1 } } */
-/* { dg-final { scan-assembler-times "\[\t \]0x6000003\[\t \]+\[^\n\]*btt_info" 1 } } */
+/* { dg-final { scan-assembler-times "\[\t \]0x86000003\[\t \]+\[^\n\]*btt_info" 1 } } */
 /* { dg-final { scan-assembler-times "ascii \"QAD.0\"\[\t \]+\[^\n\]*btf_string" 1 } } */
 /* { dg-final { scan-assembler-times "ascii \"QED.0\"\[\t \]+\[^\n\]*btf_string" 1 } } */
 /* { dg-final { scan-assembler-times "ascii \"QOD.0\"\[\t \]+\[^\n\]*btf_string" 1 } } */
diff --git a/gcc/testsuite/gcc.dg/debug/btf/btf-enum64-1.c b/gcc/testsuite/gcc.dg/debug/btf/btf-enum64-1.c
new file mode 100644
index 00000000000..da103842807
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/debug/btf/btf-enum64-1.c
@@ -0,0 +1,41 @@
+/* Test BTF generation for 64 bits enums.  */
+
+/* { dg-do compile } */
+/* { dg-options "-O0 -gbtf -dA" } */
+
+/* { dg-final { scan-assembler-times "\[\t \].size\[\t \]myenum1,\[\t \]8" 1 } } */
+/* { dg-final { scan-assembler-times "\[\t \].size\[\t \]myenum2,\[\t \]8" 1 } } */
+/* { dg-final { scan-assembler-times "\[\t \].size\[\t \]myenum3,\[\t \]8" 1 } } */
+/* { dg-final { scan-assembler-times "\[\t \]0x13000003\[\t \]+\[^\n\]*btt_info" 2 } } */
+/* { dg-final { scan-assembler-times "\[\t \]0x93000003\[\t \]+\[^\n\]*btt_info" 1 } } */
+/* { dg-final { scan-assembler-times "ascii \"B1.0\"\[\t \]+\[^\n\]*btf_string" 1 } } */
+/* { dg-final { scan-assembler-times "ascii \"B2.0\"\[\t \]+\[^\n\]*btf_string" 1 } } */
+/* { dg-final { scan-assembler-times "ascii \"B3.0\"\[\t \]+\[^\n\]*btf_string" 1 } } */
+/* { dg-final { scan-assembler-times "ascii \"C1.0\"\[\t \]+\[^\n\]*btf_string" 1 } } */
+/* { dg-final { scan-assembler-times "ascii \"C2.0\"\[\t \]+\[^\n\]*btf_string" 1 } } */
+/* { dg-final { scan-assembler-times "ascii \"C3.0\"\[\t \]+\[^\n\]*btf_string" 1 } } */
+/* { dg-final { scan-assembler-times "ascii \"D1.0\"\[\t \]+\[^\n\]*btf_string" 1 } } */
+/* { dg-final { scan-assembler-times "ascii \"D2.0\"\[\t \]+\[^\n\]*btf_string" 1 } } */
+/* { dg-final { scan-assembler-times "ascii \"D3.0\"\[\t \]+\[^\n\]*btf_string" 1 } } */
+/* { dg-final { scan-assembler-times "bte_value" 9 } } */
+
+enum default_enum
+{
+  B1 = 0xffffffffaa,
+  B2 = 0xbbbbbbbb,
+  B3 = 0xaabbccdd,
+} myenum1 = B1;
+
+enum explicit_unsigned
+{
+  C1 = 0xffffffffbbUL,
+  C2 = 0xbbbbbbbb,
+  C3 = 0xaabbccdd,
+} myenum2 = C1;
+
+enum signed64
+{
+  D1 = 0xffffffffaa,
+  D2 = 0xbbbbbbbb,
+  D3 = -0x1,
+} myenum3 = D1;
diff --git a/include/btf.h b/include/btf.h
index 78b551ced23..eba67f9d599 100644
--- a/include/btf.h
+++ b/include/btf.h
@@ -109,7 +109,8 @@ struct btf_type
 #define BTF_KIND_VAR		14	/* Variable.  */
 #define BTF_KIND_DATASEC	15	/* Section such as .bss or .data.  */
 #define BTF_KIND_FLOAT		16	/* Floating point.  */
-#define BTF_KIND_MAX		BTF_KIND_FLOAT
+#define BTF_KIND_ENUM64 	19	/* Enumeration up to 64 bits.  */
+#define BTF_KIND_MAX		BTF_KIND_ENUM64
 #define NR_BTF_KINDS		(BTF_KIND_MAX + 1)
 
 /* For some BTF_KINDs, struct btf_type is immediately followed by
@@ -130,14 +131,17 @@ struct btf_type
 #define BTF_INT_BOOL	(1 << 2)
 
 /* BTF_KIND_ENUM is followed by VLEN struct btf_enum entries,
-   which describe the enumerators. Note that BTF currently only
-   supports signed 32-bit enumerator values.  */
+   which describe the enumerators. */
 struct btf_enum
 {
   uint32_t name_off;	/* Offset in string section of enumerator name.  */
   int32_t  val;		/* Enumerator value.  */
 };
 
+/* BTF_KF_ENUM_ holds the flags for kflags in BTF_KIND_ENUM{,64}.  */
+#define BTF_KF_ENUM_UNSIGNED	(0)
+#define BTF_KF_ENUM_SIGNED 	(1 << 0)
+
 /* BTF_KIND_ARRAY is followed by a single struct btf_array.  */
 struct btf_array
 {
@@ -190,6 +194,15 @@ struct btf_var_secinfo
   uint32_t size;	/* Size (in bytes) of variable.  */
 };
 
+/* BTF_KIND_ENUM64 is followed by VLEN struct btf_enum64 entries,
+   which describe the 64 bits enumerators.  */
+struct btf_enum64
+{
+  uint32_t name_off;	/* Offset in string section of enumerator name.  */
+  uint32_t val_lo32;	/* lower 32-bit value for a 64-bit value Enumerator */
+  uint32_t val_hi32;	/* high 32-bit value for a 64-bit value Enumerator */
+};
+
 #ifdef	__cplusplus
 }
 #endif
-- 
2.35.1


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

* Re: [PATCH] btf: Add support to BTF_KIND_ENUM64 type
  2022-08-29 21:11 [PATCH] btf: Add support to BTF_KIND_ENUM64 type Guillermo E. Martinez
@ 2022-09-20 19:32 ` Guillermo E. Martinez
  2022-09-28 18:45 ` David Faust
  2022-09-28 21:15 ` [PATCH v2] " Guillermo E. Martinez
  2 siblings, 0 replies; 15+ messages in thread
From: Guillermo E. Martinez @ 2022-09-20 19:32 UTC (permalink / raw)
  To: gcc-patches, david.faust

ping


On 8/29/22 16:11, Guillermo E. Martinez wrote:
> Hello GCC team,
> 
> The following patch update BTF/CTF backend to support
> BTF_KIND_ENUM64 type.
> 
> Comments will be welcomed and appreciated!,
> 
> Kind regards,
> guillermo
> --
> 
> BTF supports 64-bits enumerators with following encoding:
> 
>    struct btf_type:
>      name_off: 0 or offset to a valid C identifier
>      info.kind_flag: 0 for unsigned, 1 for signed
>      info.kind: BTF_KIND_ENUM64
>      info.vlen: number of enum values
>      size: 1/2/4/8
> 
> The btf_type is followed by info.vlen number of:
> 
>      struct btf_enum64
>      {
>        uint32_t name_off;   /* Offset in string section of enumerator name.  */
>        uint32_t val_lo32;   /* lower 32-bit value for a 64-bit value Enumerator */
>        uint32_t val_hi32;   /* high 32-bit value for a 64-bit value Enumerator */
>      };
> 
> So, a new btf_enum64 structure was added to represent BTF_KIND_ENUM64
> and a new field in ctf_dtdef to represent specific type's properties, in
> the particular case for CTF enums it helps to distinguish when its
> enumerators values are signed or unsigned, later that information is
> used to encode the BTF enum type.
> 
> gcc/ChangeLog:
> 
> 	* btfout.cc (btf_calc_num_vbytes): Compute enumeration size depending of
> 	enumerator type btf_enum{,64}.
> 	(btf_asm_type): Update btf_kflag according to enumerators sign,
> 	using correct BPF type in BTF_KIND_ENUMi{,64}.
> 	(btf_asm_enum_const): New argument to represent the size of
> 	the BTF enum type.
> 	* ctfc.cc (ctf_add_enum): Use and initialization of flag field to
> 	CTF_ENUM_F_NONE.
> 	(ctf_add_enumerator): New argument to represent CTF flags,
> 	updating the comment and flag vaue according to enumerators
> 	sing.
> 	* ctfc.h (ctf_dmdef): Update dmd_value to HOST_WIDE_INT to allow
> 	use 32/64 bits enumerators.
> 	(ctf_dtdef): Add flags to to describe specifyc type's properties.
> 	* dwarf2ctf.cc (gen_ctf_enumeration_type): Update flags field
> 	depending when a signed enumerator value is found.
> 	include/btf.h (btf_enum64): Add new definition and new symbolic
> 	constant to BTF_KIND_ENUM64 and BTF_KF_ENUM_{UN,}SIGNED.
> 
> gcc/testsuite/ChangeLog:
> 
> 	gcc.dg/debug/btf/btf-enum-1.c: Update testcase, with correct
> 	info.kflags encoding.
> 	gcc.dg/debug/btf/btf-enum64-1.c: New testcase.
> ---
>   gcc/btfout.cc                                 | 24 ++++++++---
>   gcc/ctfc.cc                                   | 14 ++++---
>   gcc/ctfc.h                                    |  9 +++-
>   gcc/dwarf2ctf.cc                              |  9 +++-
>   gcc/testsuite/gcc.dg/debug/btf/btf-enum-1.c   |  2 +-
>   gcc/testsuite/gcc.dg/debug/btf/btf-enum64-1.c | 41 +++++++++++++++++++
>   include/btf.h                                 | 19 +++++++--
>   7 files changed, 99 insertions(+), 19 deletions(-)
>   create mode 100644 gcc/testsuite/gcc.dg/debug/btf/btf-enum64-1.c
> 
> diff --git a/gcc/btfout.cc b/gcc/btfout.cc
> index 997a33fa089..4b11c867c23 100644
> --- a/gcc/btfout.cc
> +++ b/gcc/btfout.cc
> @@ -223,7 +223,9 @@ btf_calc_num_vbytes (ctf_dtdef_ref dtd)
>         break;
>   
>       case BTF_KIND_ENUM:
> -      vlen_bytes += vlen * sizeof (struct btf_enum);
> +      vlen_bytes += (dtd->dtd_data.ctti_size == 0x8)
> +			? vlen * sizeof (struct btf_enum64)
> +			: vlen * sizeof (struct btf_enum);
>         break;
>   
>       case BTF_KIND_FUNC_PROTO:
> @@ -622,6 +624,15 @@ btf_asm_type (ctf_container_ref ctfc, ctf_dtdef_ref dtd)
>         btf_size_type = 0;
>       }
>   
> + if (btf_kind == BTF_KIND_ENUM)
> +   {
> +     btf_kflag = (dtd->flags & CTF_ENUM_F_ENUMERATORS_SIGNED)
> +		    ? BTF_KF_ENUM_SIGNED
> +		    : BTF_KF_ENUM_UNSIGNED;
> +     if (dtd->dtd_data.ctti_size == 0x8)
> +       btf_kind = BTF_KIND_ENUM64;
> +   }
> +
>     dw2_asm_output_data (4, dtd->dtd_data.ctti_name, "btt_name");
>     dw2_asm_output_data (4, BTF_TYPE_INFO (btf_kind, btf_kflag, btf_vlen),
>   		       "btt_info: kind=%u, kflag=%u, vlen=%u",
> @@ -634,6 +645,7 @@ btf_asm_type (ctf_container_ref ctfc, ctf_dtdef_ref dtd)
>       case BTF_KIND_UNION:
>       case BTF_KIND_ENUM:
>       case BTF_KIND_DATASEC:
> +    case BTF_KIND_ENUM64:
>         dw2_asm_output_data (4, dtd->dtd_data.ctti_size, "btt_size: %uB",
>   			   dtd->dtd_data.ctti_size);
>         return;
> @@ -707,13 +719,13 @@ btf_asm_sou_member (ctf_container_ref ctfc, ctf_dmdef_t * dmd)
>       }
>   }
>   
> -/* Asm'out an enum constant following a BTF_KIND_ENUM.  */
> +/* Asm'out an enum constant following a BTF_KIND_ENUM{,64}.  */
>   
>   static void
> -btf_asm_enum_const (ctf_dmdef_t * dmd)
> +btf_asm_enum_const (unsigned int size, ctf_dmdef_t * dmd)
>   {
>     dw2_asm_output_data (4, dmd->dmd_name_offset, "bte_name");
> -  dw2_asm_output_data (4, dmd->dmd_value, "bte_value");
> +  dw2_asm_output_data (size, dmd->dmd_value, "bte_value");
>   }
>   
>   /* Asm'out a function parameter description following a BTF_KIND_FUNC_PROTO.  */
> @@ -871,7 +883,7 @@ output_asm_btf_sou_fields (ctf_container_ref ctfc, ctf_dtdef_ref dtd)
>         btf_asm_sou_member (ctfc, dmd);
>   }
>   
> -/* Output all enumerator constants following a BTF_KIND_ENUM.  */
> +/* Output all enumerator constants following a BTF_KIND_ENUM{,64}.  */
>   
>   static void
>   output_asm_btf_enum_list (ctf_container_ref ARG_UNUSED (ctfc),
> @@ -881,7 +893,7 @@ output_asm_btf_enum_list (ctf_container_ref ARG_UNUSED (ctfc),
>   
>     for (dmd = dtd->dtd_u.dtu_members;
>          dmd != NULL; dmd = (ctf_dmdef_t *) ctf_dmd_list_next (dmd))
> -    btf_asm_enum_const (dmd);
> +    btf_asm_enum_const (dtd->dtd_data.ctti_size, dmd);
>   }
>   
>   /* Output all function arguments following a BTF_KIND_FUNC_PROTO.  */
> diff --git a/gcc/ctfc.cc b/gcc/ctfc.cc
> index 9773358a475..253c36b6a0a 100644
> --- a/gcc/ctfc.cc
> +++ b/gcc/ctfc.cc
> @@ -604,6 +604,7 @@ ctf_add_enum (ctf_container_ref ctfc, uint32_t flag, const char * name,
>     gcc_assert (size <= CTF_MAX_SIZE);
>   
>     dtd->dtd_data.ctti_size = size;
> +  dtd->flags = CTF_ENUM_F_NONE;
>   
>     ctfc->ctfc_num_stypes++;
>   
> @@ -612,7 +613,7 @@ ctf_add_enum (ctf_container_ref ctfc, uint32_t flag, const char * name,
>   
>   int
>   ctf_add_enumerator (ctf_container_ref ctfc, ctf_id_t enid, const char * name,
> -		    HOST_WIDE_INT value, dw_die_ref die)
> +		    HOST_WIDE_INT value, uint32_t flags, dw_die_ref die)
>   {
>     ctf_dmdef_t * dmd;
>     uint32_t kind, vlen, root;
> @@ -630,10 +631,12 @@ ctf_add_enumerator (ctf_container_ref ctfc, ctf_id_t enid, const char * name,
>   
>     gcc_assert (kind == CTF_K_ENUM && vlen < CTF_MAX_VLEN);
>   
> -  /* Enum value is of type HOST_WIDE_INT in the compiler, dmd_value is int32_t
> -     on the other hand.  Check bounds and skip adding this enum value if out of
> -     bounds.  */
> -  if ((value > INT_MAX) || (value < INT_MIN))
> +  /* Enum value is of type HOST_WIDE_INT in the compiler, CTF enumerators
> +     values in ctf_enum_t is limited to int32_t, BTF supports signed and
> +     unsigned enumerators values of 32 and 64 bits, for both debug formats
> +     we use ctf_dmdef_t.dmd_value entry of HOST_WIDE_INT type. So check
> +     CTF bounds and skip adding this enum value if out of bounds.  */
> +  if (!btf_debuginfo_p() && ((value > INT_MAX) || (value < INT_MIN)))
>       {
>         /* FIXME - Note this TBD_CTF_REPRESENTATION_LIMIT.  */
>         return (1);
> @@ -649,6 +652,7 @@ ctf_add_enumerator (ctf_container_ref ctfc, ctf_id_t enid, const char * name,
>     dmd->dmd_value = value;
>   
>     dtd->dtd_data.ctti_info = CTF_TYPE_INFO (kind, root, vlen + 1);
> +  dtd->flags |= flags;
>     ctf_dmd_list_append (&dtd->dtd_u.dtu_members, dmd);
>   
>     if ((name != NULL) && strcmp (name, ""))
> diff --git a/gcc/ctfc.h b/gcc/ctfc.h
> index bcf3a43ae1b..a22342b2610 100644
> --- a/gcc/ctfc.h
> +++ b/gcc/ctfc.h
> @@ -125,6 +125,10 @@ typedef struct GTY (()) ctf_itype
>   
>   #define CTF_FUNC_VARARG 0x1
>   
> +/* Enum specific flags.  */
> +#define CTF_ENUM_F_NONE                     (0)
> +#define CTF_ENUM_F_ENUMERATORS_SIGNED       (1 << 0)
> +
>   /* Struct/union/enum member definition for CTF generation.  */
>   
>   typedef struct GTY ((chain_next ("%h.dmd_next"))) ctf_dmdef
> @@ -133,7 +137,7 @@ typedef struct GTY ((chain_next ("%h.dmd_next"))) ctf_dmdef
>     ctf_id_t dmd_type;		/* Type of this member (for sou).  */
>     uint32_t dmd_name_offset;	/* Offset of the name in str table.  */
>     uint64_t dmd_offset;		/* Offset of this member in bits (for sou).  */
> -  int dmd_value;		/* Value of this member (for enum).  */
> +  HOST_WIDE_INT dmd_value;	/* Value of this member (for enum).  */
>     struct ctf_dmdef * dmd_next;	/* A list node.  */
>   } ctf_dmdef_t;
>   
> @@ -162,6 +166,7 @@ struct GTY ((for_user)) ctf_dtdef
>     bool from_global_func; /* Whether this type was added from a global
>   			    function.  */
>     uint32_t linkage;           /* Used in function types.  0=local, 1=global.  */
> +  uint32_t flags;             /* Flags to describe specific type's properties.  */
>     union GTY ((desc ("ctf_dtu_d_union_selector (&%1)")))
>     {
>       /* struct, union, or enum.  */
> @@ -429,7 +434,7 @@ extern ctf_id_t ctf_add_sou (ctf_container_ref, uint32_t, const char *,
>   			     uint32_t, size_t, dw_die_ref);
>   
>   extern int ctf_add_enumerator (ctf_container_ref, ctf_id_t, const char *,
> -			       HOST_WIDE_INT, dw_die_ref);
> +			       HOST_WIDE_INT, uint32_t, dw_die_ref);
>   extern int ctf_add_member_offset (ctf_container_ref, dw_die_ref, const char *,
>   				  ctf_id_t, uint64_t);
>   extern int ctf_add_function_arg (ctf_container_ref, dw_die_ref,
> diff --git a/gcc/dwarf2ctf.cc b/gcc/dwarf2ctf.cc
> index 397100004c2..0ef96dd48fd 100644
> --- a/gcc/dwarf2ctf.cc
> +++ b/gcc/dwarf2ctf.cc
> @@ -772,6 +772,7 @@ gen_ctf_enumeration_type (ctf_container_ref ctfc, dw_die_ref enumeration)
>   	  const char *enumerator_name;
>   	  dw_attr_node *enumerator_value;
>   	  HOST_WIDE_INT value_wide_int;
> +	  uint32_t flags = 0;
>   
>   	  c = dw_get_die_sib (c);
>   
> @@ -785,10 +786,14 @@ gen_ctf_enumeration_type (ctf_container_ref ctfc, dw_die_ref enumeration)
>   		  == dw_val_class_unsigned_const_implicit))
>   	    value_wide_int = AT_unsigned (enumerator_value);
>   	  else
> -	    value_wide_int = AT_int (enumerator_value);
> +	    {
> +	      value_wide_int = AT_int (enumerator_value);
> +	      flags |= CTF_ENUM_F_ENUMERATORS_SIGNED;
> +	    }
>   
>   	  ctf_add_enumerator (ctfc, enumeration_type_id,
> -			      enumerator_name, value_wide_int, enumeration);
> +			      enumerator_name, value_wide_int,
> +			      flags, enumeration);
>   	}
>         while (c != dw_get_die_child (enumeration));
>     }
> diff --git a/gcc/testsuite/gcc.dg/debug/btf/btf-enum-1.c b/gcc/testsuite/gcc.dg/debug/btf/btf-enum-1.c
> index 728493b0804..7e940529f1b 100644
> --- a/gcc/testsuite/gcc.dg/debug/btf/btf-enum-1.c
> +++ b/gcc/testsuite/gcc.dg/debug/btf/btf-enum-1.c
> @@ -4,7 +4,7 @@
>   /* { dg-options "-O0 -gbtf -dA" } */
>   
>   /* { dg-final { scan-assembler-times "\[\t \]0x6000004\[\t \]+\[^\n\]*btt_info" 1 } } */
> -/* { dg-final { scan-assembler-times "\[\t \]0x6000003\[\t \]+\[^\n\]*btt_info" 1 } } */
> +/* { dg-final { scan-assembler-times "\[\t \]0x86000003\[\t \]+\[^\n\]*btt_info" 1 } } */
>   /* { dg-final { scan-assembler-times "ascii \"QAD.0\"\[\t \]+\[^\n\]*btf_string" 1 } } */
>   /* { dg-final { scan-assembler-times "ascii \"QED.0\"\[\t \]+\[^\n\]*btf_string" 1 } } */
>   /* { dg-final { scan-assembler-times "ascii \"QOD.0\"\[\t \]+\[^\n\]*btf_string" 1 } } */
> diff --git a/gcc/testsuite/gcc.dg/debug/btf/btf-enum64-1.c b/gcc/testsuite/gcc.dg/debug/btf/btf-enum64-1.c
> new file mode 100644
> index 00000000000..da103842807
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/debug/btf/btf-enum64-1.c
> @@ -0,0 +1,41 @@
> +/* Test BTF generation for 64 bits enums.  */
> +
> +/* { dg-do compile } */
> +/* { dg-options "-O0 -gbtf -dA" } */
> +
> +/* { dg-final { scan-assembler-times "\[\t \].size\[\t \]myenum1,\[\t \]8" 1 } } */
> +/* { dg-final { scan-assembler-times "\[\t \].size\[\t \]myenum2,\[\t \]8" 1 } } */
> +/* { dg-final { scan-assembler-times "\[\t \].size\[\t \]myenum3,\[\t \]8" 1 } } */
> +/* { dg-final { scan-assembler-times "\[\t \]0x13000003\[\t \]+\[^\n\]*btt_info" 2 } } */
> +/* { dg-final { scan-assembler-times "\[\t \]0x93000003\[\t \]+\[^\n\]*btt_info" 1 } } */
> +/* { dg-final { scan-assembler-times "ascii \"B1.0\"\[\t \]+\[^\n\]*btf_string" 1 } } */
> +/* { dg-final { scan-assembler-times "ascii \"B2.0\"\[\t \]+\[^\n\]*btf_string" 1 } } */
> +/* { dg-final { scan-assembler-times "ascii \"B3.0\"\[\t \]+\[^\n\]*btf_string" 1 } } */
> +/* { dg-final { scan-assembler-times "ascii \"C1.0\"\[\t \]+\[^\n\]*btf_string" 1 } } */
> +/* { dg-final { scan-assembler-times "ascii \"C2.0\"\[\t \]+\[^\n\]*btf_string" 1 } } */
> +/* { dg-final { scan-assembler-times "ascii \"C3.0\"\[\t \]+\[^\n\]*btf_string" 1 } } */
> +/* { dg-final { scan-assembler-times "ascii \"D1.0\"\[\t \]+\[^\n\]*btf_string" 1 } } */
> +/* { dg-final { scan-assembler-times "ascii \"D2.0\"\[\t \]+\[^\n\]*btf_string" 1 } } */
> +/* { dg-final { scan-assembler-times "ascii \"D3.0\"\[\t \]+\[^\n\]*btf_string" 1 } } */
> +/* { dg-final { scan-assembler-times "bte_value" 9 } } */
> +
> +enum default_enum
> +{
> +  B1 = 0xffffffffaa,
> +  B2 = 0xbbbbbbbb,
> +  B3 = 0xaabbccdd,
> +} myenum1 = B1;
> +
> +enum explicit_unsigned
> +{
> +  C1 = 0xffffffffbbUL,
> +  C2 = 0xbbbbbbbb,
> +  C3 = 0xaabbccdd,
> +} myenum2 = C1;
> +
> +enum signed64
> +{
> +  D1 = 0xffffffffaa,
> +  D2 = 0xbbbbbbbb,
> +  D3 = -0x1,
> +} myenum3 = D1;
> diff --git a/include/btf.h b/include/btf.h
> index 78b551ced23..eba67f9d599 100644
> --- a/include/btf.h
> +++ b/include/btf.h
> @@ -109,7 +109,8 @@ struct btf_type
>   #define BTF_KIND_VAR		14	/* Variable.  */
>   #define BTF_KIND_DATASEC	15	/* Section such as .bss or .data.  */
>   #define BTF_KIND_FLOAT		16	/* Floating point.  */
> -#define BTF_KIND_MAX		BTF_KIND_FLOAT
> +#define BTF_KIND_ENUM64 	19	/* Enumeration up to 64 bits.  */
> +#define BTF_KIND_MAX		BTF_KIND_ENUM64
>   #define NR_BTF_KINDS		(BTF_KIND_MAX + 1)
>   
>   /* For some BTF_KINDs, struct btf_type is immediately followed by
> @@ -130,14 +131,17 @@ struct btf_type
>   #define BTF_INT_BOOL	(1 << 2)
>   
>   /* BTF_KIND_ENUM is followed by VLEN struct btf_enum entries,
> -   which describe the enumerators. Note that BTF currently only
> -   supports signed 32-bit enumerator values.  */
> +   which describe the enumerators. */
>   struct btf_enum
>   {
>     uint32_t name_off;	/* Offset in string section of enumerator name.  */
>     int32_t  val;		/* Enumerator value.  */
>   };
>   
> +/* BTF_KF_ENUM_ holds the flags for kflags in BTF_KIND_ENUM{,64}.  */
> +#define BTF_KF_ENUM_UNSIGNED	(0)
> +#define BTF_KF_ENUM_SIGNED 	(1 << 0)
> +
>   /* BTF_KIND_ARRAY is followed by a single struct btf_array.  */
>   struct btf_array
>   {
> @@ -190,6 +194,15 @@ struct btf_var_secinfo
>     uint32_t size;	/* Size (in bytes) of variable.  */
>   };
>   
> +/* BTF_KIND_ENUM64 is followed by VLEN struct btf_enum64 entries,
> +   which describe the 64 bits enumerators.  */
> +struct btf_enum64
> +{
> +  uint32_t name_off;	/* Offset in string section of enumerator name.  */
> +  uint32_t val_lo32;	/* lower 32-bit value for a 64-bit value Enumerator */
> +  uint32_t val_hi32;	/* high 32-bit value for a 64-bit value Enumerator */
> +};
> +
>   #ifdef	__cplusplus
>   }
>   #endif

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

* Re: [PATCH] btf: Add support to BTF_KIND_ENUM64 type
  2022-08-29 21:11 [PATCH] btf: Add support to BTF_KIND_ENUM64 type Guillermo E. Martinez
  2022-09-20 19:32 ` Guillermo E. Martinez
@ 2022-09-28 18:45 ` David Faust
  2022-09-28 21:07   ` Guillermo E. Martinez
  2022-09-28 21:15 ` [PATCH v2] " Guillermo E. Martinez
  2 siblings, 1 reply; 15+ messages in thread
From: David Faust @ 2022-09-28 18:45 UTC (permalink / raw)
  To: Guillermo E. Martinez; +Cc: gcc-patches, Indu Bhagat

Hi Guillermo,

Thanks for the patch. Just a couple of small nits on the changelog
entries below but otherwise very nice, LGTM.

But, please wait a couple of days before pushing to give Indu time
to raise any objections about the changes in ctfc/dwarf2ctf.

Thanks!
David

On 8/29/22 14:11, Guillermo E. Martinez via Gcc-patches wrote:
> Hello GCC team,
> 
> The following patch update BTF/CTF backend to support
> BTF_KIND_ENUM64 type.
> 
> Comments will be welcomed and appreciated!,
> 
> Kind regards,
> guillermo
> --
> 
> BTF supports 64-bits enumerators with following encoding:
> 
>   struct btf_type:
>     name_off: 0 or offset to a valid C identifier
>     info.kind_flag: 0 for unsigned, 1 for signed
>     info.kind: BTF_KIND_ENUM64
>     info.vlen: number of enum values
>     size: 1/2/4/8
> 
> The btf_type is followed by info.vlen number of:
> 
>     struct btf_enum64
>     {
>       uint32_t name_off;   /* Offset in string section of enumerator name.  */
>       uint32_t val_lo32;   /* lower 32-bit value for a 64-bit value Enumerator */
>       uint32_t val_hi32;   /* high 32-bit value for a 64-bit value Enumerator */
>     };
> 
> So, a new btf_enum64 structure was added to represent BTF_KIND_ENUM64
> and a new field in ctf_dtdef to represent specific type's properties, in
> the particular case for CTF enums it helps to distinguish when its
> enumerators values are signed or unsigned, later that information is
> used to encode the BTF enum type.
> 
> gcc/ChangeLog:
> 
> 	* btfout.cc (btf_calc_num_vbytes): Compute enumeration size depending of
> 	enumerator type btf_enum{,64}.
> 	(btf_asm_type): Update btf_kflag according to enumerators sign,
> 	using correct BPF type in BTF_KIND_ENUMi{,64}.
> 	(btf_asm_enum_const): New argument to represent the size of
> 	the BTF enum type.
> 	* ctfc.cc (ctf_add_enum): Use and initialization of flag field to
> 	CTF_ENUM_F_NONE.
> 	(ctf_add_enumerator): New argument to represent CTF flags,
> 	updating the comment and flag vaue according to enumerators
> 	sing.
> 	* ctfc.h (ctf_dmdef): Update dmd_value to HOST_WIDE_INT to allow
> 	use 32/64 bits enumerators.
> 	(ctf_dtdef): Add flags to to describe specifyc type's properties.

typo: specific

> 	* dwarf2ctf.cc (gen_ctf_enumeration_type): Update flags field
> 	depending when a signed enumerator value is found.
> 	include/btf.h (btf_enum64): Add new definition and new symbolic
> 	constant to BTF_KIND_ENUM64 and BTF_KF_ENUM_{UN,}SIGNED.

Missing an * here for include/btf.h

> 
> gcc/testsuite/ChangeLog:
> 
> 	gcc.dg/debug/btf/btf-enum-1.c: Update testcase, with correct
> 	info.kflags encoding.
> 	gcc.dg/debug/btf/btf-enum64-1.c: New testcase.

Likewise for these ChangeLog entries.

You can use contrib/gcc-changelog/git_check_commit.py to check the
formatting of the entries.

> ---
>  gcc/btfout.cc                                 | 24 ++++++++---
>  gcc/ctfc.cc                                   | 14 ++++---
>  gcc/ctfc.h                                    |  9 +++-
>  gcc/dwarf2ctf.cc                              |  9 +++-
>  gcc/testsuite/gcc.dg/debug/btf/btf-enum-1.c   |  2 +-
>  gcc/testsuite/gcc.dg/debug/btf/btf-enum64-1.c | 41 +++++++++++++++++++
>  include/btf.h                                 | 19 +++++++--
>  7 files changed, 99 insertions(+), 19 deletions(-)
>  create mode 100644 gcc/testsuite/gcc.dg/debug/btf/btf-enum64-1.c
> 
> diff --git a/gcc/btfout.cc b/gcc/btfout.cc
> index 997a33fa089..4b11c867c23 100644
> --- a/gcc/btfout.cc
> +++ b/gcc/btfout.cc
> @@ -223,7 +223,9 @@ btf_calc_num_vbytes (ctf_dtdef_ref dtd)
>        break;
>  
>      case BTF_KIND_ENUM:
> -      vlen_bytes += vlen * sizeof (struct btf_enum);
> +      vlen_bytes += (dtd->dtd_data.ctti_size == 0x8)
> +			? vlen * sizeof (struct btf_enum64)
> +			: vlen * sizeof (struct btf_enum);
>        break;
>  
>      case BTF_KIND_FUNC_PROTO:
> @@ -622,6 +624,15 @@ btf_asm_type (ctf_container_ref ctfc, ctf_dtdef_ref dtd)
>        btf_size_type = 0;
>      }
>  
> + if (btf_kind == BTF_KIND_ENUM)
> +   {
> +     btf_kflag = (dtd->flags & CTF_ENUM_F_ENUMERATORS_SIGNED)
> +		    ? BTF_KF_ENUM_SIGNED
> +		    : BTF_KF_ENUM_UNSIGNED;
> +     if (dtd->dtd_data.ctti_size == 0x8)
> +       btf_kind = BTF_KIND_ENUM64;
> +   }
> +
>    dw2_asm_output_data (4, dtd->dtd_data.ctti_name, "btt_name");
>    dw2_asm_output_data (4, BTF_TYPE_INFO (btf_kind, btf_kflag, btf_vlen),
>  		       "btt_info: kind=%u, kflag=%u, vlen=%u",
> @@ -634,6 +645,7 @@ btf_asm_type (ctf_container_ref ctfc, ctf_dtdef_ref dtd)
>      case BTF_KIND_UNION:
>      case BTF_KIND_ENUM:
>      case BTF_KIND_DATASEC:
> +    case BTF_KIND_ENUM64:
>        dw2_asm_output_data (4, dtd->dtd_data.ctti_size, "btt_size: %uB",
>  			   dtd->dtd_data.ctti_size);
>        return;
> @@ -707,13 +719,13 @@ btf_asm_sou_member (ctf_container_ref ctfc, ctf_dmdef_t * dmd)
>      }
>  }
>  
> -/* Asm'out an enum constant following a BTF_KIND_ENUM.  */
> +/* Asm'out an enum constant following a BTF_KIND_ENUM{,64}.  */
>  
>  static void
> -btf_asm_enum_const (ctf_dmdef_t * dmd)
> +btf_asm_enum_const (unsigned int size, ctf_dmdef_t * dmd)
>  {
>    dw2_asm_output_data (4, dmd->dmd_name_offset, "bte_name");
> -  dw2_asm_output_data (4, dmd->dmd_value, "bte_value");
> +  dw2_asm_output_data (size, dmd->dmd_value, "bte_value");
>  }
>  
>  /* Asm'out a function parameter description following a BTF_KIND_FUNC_PROTO.  */
> @@ -871,7 +883,7 @@ output_asm_btf_sou_fields (ctf_container_ref ctfc, ctf_dtdef_ref dtd)
>        btf_asm_sou_member (ctfc, dmd);
>  }
>  
> -/* Output all enumerator constants following a BTF_KIND_ENUM.  */
> +/* Output all enumerator constants following a BTF_KIND_ENUM{,64}.  */
>  
>  static void
>  output_asm_btf_enum_list (ctf_container_ref ARG_UNUSED (ctfc),
> @@ -881,7 +893,7 @@ output_asm_btf_enum_list (ctf_container_ref ARG_UNUSED (ctfc),
>  
>    for (dmd = dtd->dtd_u.dtu_members;
>         dmd != NULL; dmd = (ctf_dmdef_t *) ctf_dmd_list_next (dmd))
> -    btf_asm_enum_const (dmd);
> +    btf_asm_enum_const (dtd->dtd_data.ctti_size, dmd);
>  }
>  
>  /* Output all function arguments following a BTF_KIND_FUNC_PROTO.  */
> diff --git a/gcc/ctfc.cc b/gcc/ctfc.cc
> index 9773358a475..253c36b6a0a 100644
> --- a/gcc/ctfc.cc
> +++ b/gcc/ctfc.cc
> @@ -604,6 +604,7 @@ ctf_add_enum (ctf_container_ref ctfc, uint32_t flag, const char * name,
>    gcc_assert (size <= CTF_MAX_SIZE);
>  
>    dtd->dtd_data.ctti_size = size;
> +  dtd->flags = CTF_ENUM_F_NONE;
>  
>    ctfc->ctfc_num_stypes++;
>  
> @@ -612,7 +613,7 @@ ctf_add_enum (ctf_container_ref ctfc, uint32_t flag, const char * name,
>  
>  int
>  ctf_add_enumerator (ctf_container_ref ctfc, ctf_id_t enid, const char * name,
> -		    HOST_WIDE_INT value, dw_die_ref die)
> +		    HOST_WIDE_INT value, uint32_t flags, dw_die_ref die)
>  {
>    ctf_dmdef_t * dmd;
>    uint32_t kind, vlen, root;
> @@ -630,10 +631,12 @@ ctf_add_enumerator (ctf_container_ref ctfc, ctf_id_t enid, const char * name,
>  
>    gcc_assert (kind == CTF_K_ENUM && vlen < CTF_MAX_VLEN);
>  
> -  /* Enum value is of type HOST_WIDE_INT in the compiler, dmd_value is int32_t
> -     on the other hand.  Check bounds and skip adding this enum value if out of
> -     bounds.  */
> -  if ((value > INT_MAX) || (value < INT_MIN))
> +  /* Enum value is of type HOST_WIDE_INT in the compiler, CTF enumerators
> +     values in ctf_enum_t is limited to int32_t, BTF supports signed and
> +     unsigned enumerators values of 32 and 64 bits, for both debug formats
> +     we use ctf_dmdef_t.dmd_value entry of HOST_WIDE_INT type. So check
> +     CTF bounds and skip adding this enum value if out of bounds.  */
> +  if (!btf_debuginfo_p() && ((value > INT_MAX) || (value < INT_MIN)))
>      {
>        /* FIXME - Note this TBD_CTF_REPRESENTATION_LIMIT.  */
>        return (1);
> @@ -649,6 +652,7 @@ ctf_add_enumerator (ctf_container_ref ctfc, ctf_id_t enid, const char * name,
>    dmd->dmd_value = value;
>  
>    dtd->dtd_data.ctti_info = CTF_TYPE_INFO (kind, root, vlen + 1);
> +  dtd->flags |= flags;
>    ctf_dmd_list_append (&dtd->dtd_u.dtu_members, dmd);
>  
>    if ((name != NULL) && strcmp (name, ""))
> diff --git a/gcc/ctfc.h b/gcc/ctfc.h
> index bcf3a43ae1b..a22342b2610 100644
> --- a/gcc/ctfc.h
> +++ b/gcc/ctfc.h
> @@ -125,6 +125,10 @@ typedef struct GTY (()) ctf_itype
>  
>  #define CTF_FUNC_VARARG 0x1
>  
> +/* Enum specific flags.  */
> +#define CTF_ENUM_F_NONE                     (0)
> +#define CTF_ENUM_F_ENUMERATORS_SIGNED       (1 << 0)
> +
>  /* Struct/union/enum member definition for CTF generation.  */
>  
>  typedef struct GTY ((chain_next ("%h.dmd_next"))) ctf_dmdef
> @@ -133,7 +137,7 @@ typedef struct GTY ((chain_next ("%h.dmd_next"))) ctf_dmdef
>    ctf_id_t dmd_type;		/* Type of this member (for sou).  */
>    uint32_t dmd_name_offset;	/* Offset of the name in str table.  */
>    uint64_t dmd_offset;		/* Offset of this member in bits (for sou).  */
> -  int dmd_value;		/* Value of this member (for enum).  */
> +  HOST_WIDE_INT dmd_value;	/* Value of this member (for enum).  */
>    struct ctf_dmdef * dmd_next;	/* A list node.  */
>  } ctf_dmdef_t;
>  
> @@ -162,6 +166,7 @@ struct GTY ((for_user)) ctf_dtdef
>    bool from_global_func; /* Whether this type was added from a global
>  			    function.  */
>    uint32_t linkage;           /* Used in function types.  0=local, 1=global.  */
> +  uint32_t flags;             /* Flags to describe specific type's properties.  */
>    union GTY ((desc ("ctf_dtu_d_union_selector (&%1)")))
>    {
>      /* struct, union, or enum.  */
> @@ -429,7 +434,7 @@ extern ctf_id_t ctf_add_sou (ctf_container_ref, uint32_t, const char *,
>  			     uint32_t, size_t, dw_die_ref);
>  
>  extern int ctf_add_enumerator (ctf_container_ref, ctf_id_t, const char *,
> -			       HOST_WIDE_INT, dw_die_ref);
> +			       HOST_WIDE_INT, uint32_t, dw_die_ref);
>  extern int ctf_add_member_offset (ctf_container_ref, dw_die_ref, const char *,
>  				  ctf_id_t, uint64_t);
>  extern int ctf_add_function_arg (ctf_container_ref, dw_die_ref,
> diff --git a/gcc/dwarf2ctf.cc b/gcc/dwarf2ctf.cc
> index 397100004c2..0ef96dd48fd 100644
> --- a/gcc/dwarf2ctf.cc
> +++ b/gcc/dwarf2ctf.cc
> @@ -772,6 +772,7 @@ gen_ctf_enumeration_type (ctf_container_ref ctfc, dw_die_ref enumeration)
>  	  const char *enumerator_name;
>  	  dw_attr_node *enumerator_value;
>  	  HOST_WIDE_INT value_wide_int;
> +	  uint32_t flags = 0;
>  
>  	  c = dw_get_die_sib (c);
>  
> @@ -785,10 +786,14 @@ gen_ctf_enumeration_type (ctf_container_ref ctfc, dw_die_ref enumeration)
>  		  == dw_val_class_unsigned_const_implicit))
>  	    value_wide_int = AT_unsigned (enumerator_value);
>  	  else
> -	    value_wide_int = AT_int (enumerator_value);
> +	    {
> +	      value_wide_int = AT_int (enumerator_value);
> +	      flags |= CTF_ENUM_F_ENUMERATORS_SIGNED;
> +	    }
>  
>  	  ctf_add_enumerator (ctfc, enumeration_type_id,
> -			      enumerator_name, value_wide_int, enumeration);
> +			      enumerator_name, value_wide_int,
> +			      flags, enumeration);
>  	}
>        while (c != dw_get_die_child (enumeration));
>    }
> diff --git a/gcc/testsuite/gcc.dg/debug/btf/btf-enum-1.c b/gcc/testsuite/gcc.dg/debug/btf/btf-enum-1.c
> index 728493b0804..7e940529f1b 100644
> --- a/gcc/testsuite/gcc.dg/debug/btf/btf-enum-1.c
> +++ b/gcc/testsuite/gcc.dg/debug/btf/btf-enum-1.c
> @@ -4,7 +4,7 @@
>  /* { dg-options "-O0 -gbtf -dA" } */
>  
>  /* { dg-final { scan-assembler-times "\[\t \]0x6000004\[\t \]+\[^\n\]*btt_info" 1 } } */
> -/* { dg-final { scan-assembler-times "\[\t \]0x6000003\[\t \]+\[^\n\]*btt_info" 1 } } */
> +/* { dg-final { scan-assembler-times "\[\t \]0x86000003\[\t \]+\[^\n\]*btt_info" 1 } } */
>  /* { dg-final { scan-assembler-times "ascii \"QAD.0\"\[\t \]+\[^\n\]*btf_string" 1 } } */
>  /* { dg-final { scan-assembler-times "ascii \"QED.0\"\[\t \]+\[^\n\]*btf_string" 1 } } */
>  /* { dg-final { scan-assembler-times "ascii \"QOD.0\"\[\t \]+\[^\n\]*btf_string" 1 } } */
> diff --git a/gcc/testsuite/gcc.dg/debug/btf/btf-enum64-1.c b/gcc/testsuite/gcc.dg/debug/btf/btf-enum64-1.c
> new file mode 100644
> index 00000000000..da103842807
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/debug/btf/btf-enum64-1.c
> @@ -0,0 +1,41 @@
> +/* Test BTF generation for 64 bits enums.  */
> +
> +/* { dg-do compile } */
> +/* { dg-options "-O0 -gbtf -dA" } */
> +
> +/* { dg-final { scan-assembler-times "\[\t \].size\[\t \]myenum1,\[\t \]8" 1 } } */
> +/* { dg-final { scan-assembler-times "\[\t \].size\[\t \]myenum2,\[\t \]8" 1 } } */
> +/* { dg-final { scan-assembler-times "\[\t \].size\[\t \]myenum3,\[\t \]8" 1 } } */
> +/* { dg-final { scan-assembler-times "\[\t \]0x13000003\[\t \]+\[^\n\]*btt_info" 2 } } */
> +/* { dg-final { scan-assembler-times "\[\t \]0x93000003\[\t \]+\[^\n\]*btt_info" 1 } } */
> +/* { dg-final { scan-assembler-times "ascii \"B1.0\"\[\t \]+\[^\n\]*btf_string" 1 } } */
> +/* { dg-final { scan-assembler-times "ascii \"B2.0\"\[\t \]+\[^\n\]*btf_string" 1 } } */
> +/* { dg-final { scan-assembler-times "ascii \"B3.0\"\[\t \]+\[^\n\]*btf_string" 1 } } */
> +/* { dg-final { scan-assembler-times "ascii \"C1.0\"\[\t \]+\[^\n\]*btf_string" 1 } } */
> +/* { dg-final { scan-assembler-times "ascii \"C2.0\"\[\t \]+\[^\n\]*btf_string" 1 } } */
> +/* { dg-final { scan-assembler-times "ascii \"C3.0\"\[\t \]+\[^\n\]*btf_string" 1 } } */
> +/* { dg-final { scan-assembler-times "ascii \"D1.0\"\[\t \]+\[^\n\]*btf_string" 1 } } */
> +/* { dg-final { scan-assembler-times "ascii \"D2.0\"\[\t \]+\[^\n\]*btf_string" 1 } } */
> +/* { dg-final { scan-assembler-times "ascii \"D3.0\"\[\t \]+\[^\n\]*btf_string" 1 } } */
> +/* { dg-final { scan-assembler-times "bte_value" 9 } } */
> +
> +enum default_enum
> +{
> +  B1 = 0xffffffffaa,
> +  B2 = 0xbbbbbbbb,
> +  B3 = 0xaabbccdd,
> +} myenum1 = B1;
> +
> +enum explicit_unsigned
> +{
> +  C1 = 0xffffffffbbUL,
> +  C2 = 0xbbbbbbbb,
> +  C3 = 0xaabbccdd,
> +} myenum2 = C1;
> +
> +enum signed64
> +{
> +  D1 = 0xffffffffaa,
> +  D2 = 0xbbbbbbbb,
> +  D3 = -0x1,
> +} myenum3 = D1;
> diff --git a/include/btf.h b/include/btf.h
> index 78b551ced23..eba67f9d599 100644
> --- a/include/btf.h
> +++ b/include/btf.h
> @@ -109,7 +109,8 @@ struct btf_type
>  #define BTF_KIND_VAR		14	/* Variable.  */
>  #define BTF_KIND_DATASEC	15	/* Section such as .bss or .data.  */
>  #define BTF_KIND_FLOAT		16	/* Floating point.  */
> -#define BTF_KIND_MAX		BTF_KIND_FLOAT
> +#define BTF_KIND_ENUM64 	19	/* Enumeration up to 64 bits.  */
> +#define BTF_KIND_MAX		BTF_KIND_ENUM64
>  #define NR_BTF_KINDS		(BTF_KIND_MAX + 1)
>  
>  /* For some BTF_KINDs, struct btf_type is immediately followed by
> @@ -130,14 +131,17 @@ struct btf_type
>  #define BTF_INT_BOOL	(1 << 2)
>  
>  /* BTF_KIND_ENUM is followed by VLEN struct btf_enum entries,
> -   which describe the enumerators. Note that BTF currently only
> -   supports signed 32-bit enumerator values.  */
> +   which describe the enumerators. */
>  struct btf_enum
>  {
>    uint32_t name_off;	/* Offset in string section of enumerator name.  */
>    int32_t  val;		/* Enumerator value.  */
>  };
>  
> +/* BTF_KF_ENUM_ holds the flags for kflags in BTF_KIND_ENUM{,64}.  */
> +#define BTF_KF_ENUM_UNSIGNED	(0)
> +#define BTF_KF_ENUM_SIGNED 	(1 << 0)
> +
>  /* BTF_KIND_ARRAY is followed by a single struct btf_array.  */
>  struct btf_array
>  {
> @@ -190,6 +194,15 @@ struct btf_var_secinfo
>    uint32_t size;	/* Size (in bytes) of variable.  */
>  };
>  
> +/* BTF_KIND_ENUM64 is followed by VLEN struct btf_enum64 entries,
> +   which describe the 64 bits enumerators.  */
> +struct btf_enum64
> +{
> +  uint32_t name_off;	/* Offset in string section of enumerator name.  */
> +  uint32_t val_lo32;	/* lower 32-bit value for a 64-bit value Enumerator */
> +  uint32_t val_hi32;	/* high 32-bit value for a 64-bit value Enumerator */
> +};
> +
>  #ifdef	__cplusplus
>  }
>  #endif

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

* Re: [PATCH] btf: Add support to BTF_KIND_ENUM64 type
  2022-09-28 18:45 ` David Faust
@ 2022-09-28 21:07   ` Guillermo E. Martinez
  0 siblings, 0 replies; 15+ messages in thread
From: Guillermo E. Martinez @ 2022-09-28 21:07 UTC (permalink / raw)
  To: David Faust; +Cc: gcc-patches, Indu Bhagat



On 9/28/22 13:45, David Faust wrote:
> Hi Guillermo,
> 

Hi David,

> Thanks for the patch. Just a couple of small nits on the changelog
> entries below but otherwise very nice, LGTM.
> 
> But, please wait a couple of days before pushing to give Indu time
> to raise any objections about the changes in ctfc/dwarf2ctf.
>

OK. Thanks for your comments!.
  
> Thanks!
> David
> 
> On 8/29/22 14:11, Guillermo E. Martinez via Gcc-patches wrote:
>> Hello GCC team,
>>
>> The following patch update BTF/CTF backend to support
>> BTF_KIND_ENUM64 type.
>>
>> Comments will be welcomed and appreciated!,
>>
>> Kind regards,
>> guillermo
>> --
>>
>> BTF supports 64-bits enumerators with following encoding:
>>
>>    struct btf_type:
>>      name_off: 0 or offset to a valid C identifier
>>      info.kind_flag: 0 for unsigned, 1 for signed
>>      info.kind: BTF_KIND_ENUM64
>>      info.vlen: number of enum values
>>      size: 1/2/4/8
>>
>> The btf_type is followed by info.vlen number of:
>>
>>      struct btf_enum64
>>      {
>>        uint32_t name_off;   /* Offset in string section of enumerator name.  */
>>        uint32_t val_lo32;   /* lower 32-bit value for a 64-bit value Enumerator */
>>        uint32_t val_hi32;   /* high 32-bit value for a 64-bit value Enumerator */
>>      };
>>
>> So, a new btf_enum64 structure was added to represent BTF_KIND_ENUM64
>> and a new field in ctf_dtdef to represent specific type's properties, in
>> the particular case for CTF enums it helps to distinguish when its
>> enumerators values are signed or unsigned, later that information is
>> used to encode the BTF enum type.
>>
>> gcc/ChangeLog:
>>
>> 	* btfout.cc (btf_calc_num_vbytes): Compute enumeration size depending of
>> 	enumerator type btf_enum{,64}.
>> 	(btf_asm_type): Update btf_kflag according to enumerators sign,
>> 	using correct BPF type in BTF_KIND_ENUMi{,64}.
>> 	(btf_asm_enum_const): New argument to represent the size of
>> 	the BTF enum type.
>> 	* ctfc.cc (ctf_add_enum): Use and initialization of flag field to
>> 	CTF_ENUM_F_NONE.
>> 	(ctf_add_enumerator): New argument to represent CTF flags,
>> 	updating the comment and flag vaue according to enumerators
>> 	sing.
>> 	* ctfc.h (ctf_dmdef): Update dmd_value to HOST_WIDE_INT to allow
>> 	use 32/64 bits enumerators.
>> 	(ctf_dtdef): Add flags to to describe specifyc type's properties.
> 
> typo: specific
> 

Fixed in v2.

>> 	* dwarf2ctf.cc (gen_ctf_enumeration_type): Update flags field
>> 	depending when a signed enumerator value is found.
>> 	include/btf.h (btf_enum64): Add new definition and new symbolic
>> 	constant to BTF_KIND_ENUM64 and BTF_KF_ENUM_{UN,}SIGNED.
> 
> Missing an * here for include/btf.h
> 

Fixed in v2.

>>
>> gcc/testsuite/ChangeLog:
>>
>> 	gcc.dg/debug/btf/btf-enum-1.c: Update testcase, with correct
>> 	info.kflags encoding.
>> 	gcc.dg/debug/btf/btf-enum64-1.c: New testcase.
> 
> Likewise for these ChangeLog entries.
> 

Fixed in v2.

> You can use contrib/gcc-changelog/git_check_commit.py to check the
> formatting of the entries.
> 

Oh. thanks for mention it, really useful.

>> ---
>>   gcc/btfout.cc                                 | 24 ++++++++---
>>   gcc/ctfc.cc                                   | 14 ++++---
>>   gcc/ctfc.h                                    |  9 +++-
>>   gcc/dwarf2ctf.cc                              |  9 +++-
>>   gcc/testsuite/gcc.dg/debug/btf/btf-enum-1.c   |  2 +-
>>   gcc/testsuite/gcc.dg/debug/btf/btf-enum64-1.c | 41 +++++++++++++++++++
>>   include/btf.h                                 | 19 +++++++--
>>   7 files changed, 99 insertions(+), 19 deletions(-)
>>   create mode 100644 gcc/testsuite/gcc.dg/debug/btf/btf-enum64-1.c
>>
>> diff --git a/gcc/btfout.cc b/gcc/btfout.cc
>> index 997a33fa089..4b11c867c23 100644
>> --- a/gcc/btfout.cc
>>[...]

Regards,
guillermo

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

* [PATCH v2] btf: Add support to BTF_KIND_ENUM64 type
  2022-08-29 21:11 [PATCH] btf: Add support to BTF_KIND_ENUM64 type Guillermo E. Martinez
  2022-09-20 19:32 ` Guillermo E. Martinez
  2022-09-28 18:45 ` David Faust
@ 2022-09-28 21:15 ` Guillermo E. Martinez
  2022-09-29 22:35   ` Indu Bhagat
  2022-10-15  3:55   ` [PATCH v3] " Guillermo E. Martinez
  2 siblings, 2 replies; 15+ messages in thread
From: Guillermo E. Martinez @ 2022-09-28 21:15 UTC (permalink / raw)
  To: gcc-patches; +Cc: david.faust, indu.bhagat, Guillermo E. Martinez

Hello GCC team,

The following is patch v2 to update BTF/CTF backend supporting
BTF_KIND_ENUM64 type. Changes from v1:

  + Fix typo in commit message.
  + Fix changelog entries.

Comments will be welcomed and appreciated!,

Kind regards,
guillermo
--

BTF supports 64-bits enumerators with following encoding:

  struct btf_type:
    name_off: 0 or offset to a valid C identifier
    info.kind_flag: 0 for unsigned, 1 for signed
    info.kind: BTF_KIND_ENUM64
    info.vlen: number of enum values
    size: 1/2/4/8

The btf_type is followed by info.vlen number of:

    struct btf_enum64
    {
      uint32_t name_off;   /* Offset in string section of enumerator name.  */
      uint32_t val_lo32;   /* lower 32-bit value for a 64-bit value Enumerator */
      uint32_t val_hi32;   /* high 32-bit value for a 64-bit value Enumerator */
    };

So, a new btf_enum64 structure was added to represent BTF_KIND_ENUM64
and a new field in ctf_dtdef to represent specific type's properties, in
the particular case for CTF enums it helps to distinguish when its
enumerators values are signed or unsigned, later that information is
used to encode the BTF enum type.

gcc/ChangeLog:

	* btfout.cc (btf_calc_num_vbytes): Compute enumeration size depending of
	enumerator type btf_enum{,64}.
	(btf_asm_type): Update btf_kflag according to enumerators sign,
	using correct BPF type in BTF_KIND_ENUMi{,64}.
	(btf_asm_enum_const): New argument to represent the size of
	the BTF enum type.
	* ctfc.cc (ctf_add_enum): Use and initialization of flag field to
	CTF_ENUM_F_NONE.
	(ctf_add_enumerator): New argument to represent CTF flags,
	updating the comment and flag vaue according to enumerators
	sing.
	* ctfc.h (ctf_dmdef): Update dmd_value to HOST_WIDE_INT to allow
	use 32/64 bits enumerators.
	(ctf_dtdef): Add flags to to describe specific type's properties.
	* dwarf2ctf.cc (gen_ctf_enumeration_type): Update flags field
	depending when a signed enumerator value is found.

include/
	* btf.h (btf_enum64): Add new definition and new symbolic
	constant to BTF_KIND_ENUM64 and BTF_KF_ENUM_{UN,}SIGNED.

gcc/testsuite/ChangeLog:

	* gcc.dg/debug/btf/btf-enum-1.c: Update testcase, with correct
	info.kflags encoding.
	* gcc.dg/debug/btf/btf-enum64-1.c: New testcase.
---
 gcc/btfout.cc                                 | 24 ++++++++---
 gcc/ctfc.cc                                   | 14 ++++---
 gcc/ctfc.h                                    |  9 +++-
 gcc/dwarf2ctf.cc                              |  9 +++-
 gcc/testsuite/gcc.dg/debug/btf/btf-enum-1.c   |  2 +-
 gcc/testsuite/gcc.dg/debug/btf/btf-enum64-1.c | 41 +++++++++++++++++++
 include/btf.h                                 | 19 +++++++--
 7 files changed, 99 insertions(+), 19 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/debug/btf/btf-enum64-1.c

diff --git a/gcc/btfout.cc b/gcc/btfout.cc
index 997a33fa089..4b11c867c23 100644
--- a/gcc/btfout.cc
+++ b/gcc/btfout.cc
@@ -223,7 +223,9 @@ btf_calc_num_vbytes (ctf_dtdef_ref dtd)
       break;
 
     case BTF_KIND_ENUM:
-      vlen_bytes += vlen * sizeof (struct btf_enum);
+      vlen_bytes += (dtd->dtd_data.ctti_size == 0x8)
+			? vlen * sizeof (struct btf_enum64)
+			: vlen * sizeof (struct btf_enum);
       break;
 
     case BTF_KIND_FUNC_PROTO:
@@ -622,6 +624,15 @@ btf_asm_type (ctf_container_ref ctfc, ctf_dtdef_ref dtd)
       btf_size_type = 0;
     }
 
+ if (btf_kind == BTF_KIND_ENUM)
+   {
+     btf_kflag = (dtd->flags & CTF_ENUM_F_ENUMERATORS_SIGNED)
+		    ? BTF_KF_ENUM_SIGNED
+		    : BTF_KF_ENUM_UNSIGNED;
+     if (dtd->dtd_data.ctti_size == 0x8)
+       btf_kind = BTF_KIND_ENUM64;
+   }
+
   dw2_asm_output_data (4, dtd->dtd_data.ctti_name, "btt_name");
   dw2_asm_output_data (4, BTF_TYPE_INFO (btf_kind, btf_kflag, btf_vlen),
 		       "btt_info: kind=%u, kflag=%u, vlen=%u",
@@ -634,6 +645,7 @@ btf_asm_type (ctf_container_ref ctfc, ctf_dtdef_ref dtd)
     case BTF_KIND_UNION:
     case BTF_KIND_ENUM:
     case BTF_KIND_DATASEC:
+    case BTF_KIND_ENUM64:
       dw2_asm_output_data (4, dtd->dtd_data.ctti_size, "btt_size: %uB",
 			   dtd->dtd_data.ctti_size);
       return;
@@ -707,13 +719,13 @@ btf_asm_sou_member (ctf_container_ref ctfc, ctf_dmdef_t * dmd)
     }
 }
 
-/* Asm'out an enum constant following a BTF_KIND_ENUM.  */
+/* Asm'out an enum constant following a BTF_KIND_ENUM{,64}.  */
 
 static void
-btf_asm_enum_const (ctf_dmdef_t * dmd)
+btf_asm_enum_const (unsigned int size, ctf_dmdef_t * dmd)
 {
   dw2_asm_output_data (4, dmd->dmd_name_offset, "bte_name");
-  dw2_asm_output_data (4, dmd->dmd_value, "bte_value");
+  dw2_asm_output_data (size, dmd->dmd_value, "bte_value");
 }
 
 /* Asm'out a function parameter description following a BTF_KIND_FUNC_PROTO.  */
@@ -871,7 +883,7 @@ output_asm_btf_sou_fields (ctf_container_ref ctfc, ctf_dtdef_ref dtd)
       btf_asm_sou_member (ctfc, dmd);
 }
 
-/* Output all enumerator constants following a BTF_KIND_ENUM.  */
+/* Output all enumerator constants following a BTF_KIND_ENUM{,64}.  */
 
 static void
 output_asm_btf_enum_list (ctf_container_ref ARG_UNUSED (ctfc),
@@ -881,7 +893,7 @@ output_asm_btf_enum_list (ctf_container_ref ARG_UNUSED (ctfc),
 
   for (dmd = dtd->dtd_u.dtu_members;
        dmd != NULL; dmd = (ctf_dmdef_t *) ctf_dmd_list_next (dmd))
-    btf_asm_enum_const (dmd);
+    btf_asm_enum_const (dtd->dtd_data.ctti_size, dmd);
 }
 
 /* Output all function arguments following a BTF_KIND_FUNC_PROTO.  */
diff --git a/gcc/ctfc.cc b/gcc/ctfc.cc
index 9773358a475..253c36b6a0a 100644
--- a/gcc/ctfc.cc
+++ b/gcc/ctfc.cc
@@ -604,6 +604,7 @@ ctf_add_enum (ctf_container_ref ctfc, uint32_t flag, const char * name,
   gcc_assert (size <= CTF_MAX_SIZE);
 
   dtd->dtd_data.ctti_size = size;
+  dtd->flags = CTF_ENUM_F_NONE;
 
   ctfc->ctfc_num_stypes++;
 
@@ -612,7 +613,7 @@ ctf_add_enum (ctf_container_ref ctfc, uint32_t flag, const char * name,
 
 int
 ctf_add_enumerator (ctf_container_ref ctfc, ctf_id_t enid, const char * name,
-		    HOST_WIDE_INT value, dw_die_ref die)
+		    HOST_WIDE_INT value, uint32_t flags, dw_die_ref die)
 {
   ctf_dmdef_t * dmd;
   uint32_t kind, vlen, root;
@@ -630,10 +631,12 @@ ctf_add_enumerator (ctf_container_ref ctfc, ctf_id_t enid, const char * name,
 
   gcc_assert (kind == CTF_K_ENUM && vlen < CTF_MAX_VLEN);
 
-  /* Enum value is of type HOST_WIDE_INT in the compiler, dmd_value is int32_t
-     on the other hand.  Check bounds and skip adding this enum value if out of
-     bounds.  */
-  if ((value > INT_MAX) || (value < INT_MIN))
+  /* Enum value is of type HOST_WIDE_INT in the compiler, CTF enumerators
+     values in ctf_enum_t is limited to int32_t, BTF supports signed and
+     unsigned enumerators values of 32 and 64 bits, for both debug formats
+     we use ctf_dmdef_t.dmd_value entry of HOST_WIDE_INT type. So check
+     CTF bounds and skip adding this enum value if out of bounds.  */
+  if (!btf_debuginfo_p() && ((value > INT_MAX) || (value < INT_MIN)))
     {
       /* FIXME - Note this TBD_CTF_REPRESENTATION_LIMIT.  */
       return (1);
@@ -649,6 +652,7 @@ ctf_add_enumerator (ctf_container_ref ctfc, ctf_id_t enid, const char * name,
   dmd->dmd_value = value;
 
   dtd->dtd_data.ctti_info = CTF_TYPE_INFO (kind, root, vlen + 1);
+  dtd->flags |= flags;
   ctf_dmd_list_append (&dtd->dtd_u.dtu_members, dmd);
 
   if ((name != NULL) && strcmp (name, ""))
diff --git a/gcc/ctfc.h b/gcc/ctfc.h
index bcf3a43ae1b..a22342b2610 100644
--- a/gcc/ctfc.h
+++ b/gcc/ctfc.h
@@ -125,6 +125,10 @@ typedef struct GTY (()) ctf_itype
 
 #define CTF_FUNC_VARARG 0x1
 
+/* Enum specific flags.  */
+#define CTF_ENUM_F_NONE                     (0)
+#define CTF_ENUM_F_ENUMERATORS_SIGNED       (1 << 0)
+
 /* Struct/union/enum member definition for CTF generation.  */
 
 typedef struct GTY ((chain_next ("%h.dmd_next"))) ctf_dmdef
@@ -133,7 +137,7 @@ typedef struct GTY ((chain_next ("%h.dmd_next"))) ctf_dmdef
   ctf_id_t dmd_type;		/* Type of this member (for sou).  */
   uint32_t dmd_name_offset;	/* Offset of the name in str table.  */
   uint64_t dmd_offset;		/* Offset of this member in bits (for sou).  */
-  int dmd_value;		/* Value of this member (for enum).  */
+  HOST_WIDE_INT dmd_value;	/* Value of this member (for enum).  */
   struct ctf_dmdef * dmd_next;	/* A list node.  */
 } ctf_dmdef_t;
 
@@ -162,6 +166,7 @@ struct GTY ((for_user)) ctf_dtdef
   bool from_global_func; /* Whether this type was added from a global
 			    function.  */
   uint32_t linkage;           /* Used in function types.  0=local, 1=global.  */
+  uint32_t flags;             /* Flags to describe specific type's properties.  */
   union GTY ((desc ("ctf_dtu_d_union_selector (&%1)")))
   {
     /* struct, union, or enum.  */
@@ -429,7 +434,7 @@ extern ctf_id_t ctf_add_sou (ctf_container_ref, uint32_t, const char *,
 			     uint32_t, size_t, dw_die_ref);
 
 extern int ctf_add_enumerator (ctf_container_ref, ctf_id_t, const char *,
-			       HOST_WIDE_INT, dw_die_ref);
+			       HOST_WIDE_INT, uint32_t, dw_die_ref);
 extern int ctf_add_member_offset (ctf_container_ref, dw_die_ref, const char *,
 				  ctf_id_t, uint64_t);
 extern int ctf_add_function_arg (ctf_container_ref, dw_die_ref,
diff --git a/gcc/dwarf2ctf.cc b/gcc/dwarf2ctf.cc
index 397100004c2..0ef96dd48fd 100644
--- a/gcc/dwarf2ctf.cc
+++ b/gcc/dwarf2ctf.cc
@@ -772,6 +772,7 @@ gen_ctf_enumeration_type (ctf_container_ref ctfc, dw_die_ref enumeration)
 	  const char *enumerator_name;
 	  dw_attr_node *enumerator_value;
 	  HOST_WIDE_INT value_wide_int;
+	  uint32_t flags = 0;
 
 	  c = dw_get_die_sib (c);
 
@@ -785,10 +786,14 @@ gen_ctf_enumeration_type (ctf_container_ref ctfc, dw_die_ref enumeration)
 		  == dw_val_class_unsigned_const_implicit))
 	    value_wide_int = AT_unsigned (enumerator_value);
 	  else
-	    value_wide_int = AT_int (enumerator_value);
+	    {
+	      value_wide_int = AT_int (enumerator_value);
+	      flags |= CTF_ENUM_F_ENUMERATORS_SIGNED;
+	    }
 
 	  ctf_add_enumerator (ctfc, enumeration_type_id,
-			      enumerator_name, value_wide_int, enumeration);
+			      enumerator_name, value_wide_int,
+			      flags, enumeration);
 	}
       while (c != dw_get_die_child (enumeration));
   }
diff --git a/gcc/testsuite/gcc.dg/debug/btf/btf-enum-1.c b/gcc/testsuite/gcc.dg/debug/btf/btf-enum-1.c
index 728493b0804..7e940529f1b 100644
--- a/gcc/testsuite/gcc.dg/debug/btf/btf-enum-1.c
+++ b/gcc/testsuite/gcc.dg/debug/btf/btf-enum-1.c
@@ -4,7 +4,7 @@
 /* { dg-options "-O0 -gbtf -dA" } */
 
 /* { dg-final { scan-assembler-times "\[\t \]0x6000004\[\t \]+\[^\n\]*btt_info" 1 } } */
-/* { dg-final { scan-assembler-times "\[\t \]0x6000003\[\t \]+\[^\n\]*btt_info" 1 } } */
+/* { dg-final { scan-assembler-times "\[\t \]0x86000003\[\t \]+\[^\n\]*btt_info" 1 } } */
 /* { dg-final { scan-assembler-times "ascii \"QAD.0\"\[\t \]+\[^\n\]*btf_string" 1 } } */
 /* { dg-final { scan-assembler-times "ascii \"QED.0\"\[\t \]+\[^\n\]*btf_string" 1 } } */
 /* { dg-final { scan-assembler-times "ascii \"QOD.0\"\[\t \]+\[^\n\]*btf_string" 1 } } */
diff --git a/gcc/testsuite/gcc.dg/debug/btf/btf-enum64-1.c b/gcc/testsuite/gcc.dg/debug/btf/btf-enum64-1.c
new file mode 100644
index 00000000000..da103842807
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/debug/btf/btf-enum64-1.c
@@ -0,0 +1,41 @@
+/* Test BTF generation for 64 bits enums.  */
+
+/* { dg-do compile } */
+/* { dg-options "-O0 -gbtf -dA" } */
+
+/* { dg-final { scan-assembler-times "\[\t \].size\[\t \]myenum1,\[\t \]8" 1 } } */
+/* { dg-final { scan-assembler-times "\[\t \].size\[\t \]myenum2,\[\t \]8" 1 } } */
+/* { dg-final { scan-assembler-times "\[\t \].size\[\t \]myenum3,\[\t \]8" 1 } } */
+/* { dg-final { scan-assembler-times "\[\t \]0x13000003\[\t \]+\[^\n\]*btt_info" 2 } } */
+/* { dg-final { scan-assembler-times "\[\t \]0x93000003\[\t \]+\[^\n\]*btt_info" 1 } } */
+/* { dg-final { scan-assembler-times "ascii \"B1.0\"\[\t \]+\[^\n\]*btf_string" 1 } } */
+/* { dg-final { scan-assembler-times "ascii \"B2.0\"\[\t \]+\[^\n\]*btf_string" 1 } } */
+/* { dg-final { scan-assembler-times "ascii \"B3.0\"\[\t \]+\[^\n\]*btf_string" 1 } } */
+/* { dg-final { scan-assembler-times "ascii \"C1.0\"\[\t \]+\[^\n\]*btf_string" 1 } } */
+/* { dg-final { scan-assembler-times "ascii \"C2.0\"\[\t \]+\[^\n\]*btf_string" 1 } } */
+/* { dg-final { scan-assembler-times "ascii \"C3.0\"\[\t \]+\[^\n\]*btf_string" 1 } } */
+/* { dg-final { scan-assembler-times "ascii \"D1.0\"\[\t \]+\[^\n\]*btf_string" 1 } } */
+/* { dg-final { scan-assembler-times "ascii \"D2.0\"\[\t \]+\[^\n\]*btf_string" 1 } } */
+/* { dg-final { scan-assembler-times "ascii \"D3.0\"\[\t \]+\[^\n\]*btf_string" 1 } } */
+/* { dg-final { scan-assembler-times "bte_value" 9 } } */
+
+enum default_enum
+{
+  B1 = 0xffffffffaa,
+  B2 = 0xbbbbbbbb,
+  B3 = 0xaabbccdd,
+} myenum1 = B1;
+
+enum explicit_unsigned
+{
+  C1 = 0xffffffffbbUL,
+  C2 = 0xbbbbbbbb,
+  C3 = 0xaabbccdd,
+} myenum2 = C1;
+
+enum signed64
+{
+  D1 = 0xffffffffaa,
+  D2 = 0xbbbbbbbb,
+  D3 = -0x1,
+} myenum3 = D1;
diff --git a/include/btf.h b/include/btf.h
index 78b551ced23..eba67f9d599 100644
--- a/include/btf.h
+++ b/include/btf.h
@@ -109,7 +109,8 @@ struct btf_type
 #define BTF_KIND_VAR		14	/* Variable.  */
 #define BTF_KIND_DATASEC	15	/* Section such as .bss or .data.  */
 #define BTF_KIND_FLOAT		16	/* Floating point.  */
-#define BTF_KIND_MAX		BTF_KIND_FLOAT
+#define BTF_KIND_ENUM64 	19	/* Enumeration up to 64 bits.  */
+#define BTF_KIND_MAX		BTF_KIND_ENUM64
 #define NR_BTF_KINDS		(BTF_KIND_MAX + 1)
 
 /* For some BTF_KINDs, struct btf_type is immediately followed by
@@ -130,14 +131,17 @@ struct btf_type
 #define BTF_INT_BOOL	(1 << 2)
 
 /* BTF_KIND_ENUM is followed by VLEN struct btf_enum entries,
-   which describe the enumerators. Note that BTF currently only
-   supports signed 32-bit enumerator values.  */
+   which describe the enumerators. */
 struct btf_enum
 {
   uint32_t name_off;	/* Offset in string section of enumerator name.  */
   int32_t  val;		/* Enumerator value.  */
 };
 
+/* BTF_KF_ENUM_ holds the flags for kflags in BTF_KIND_ENUM{,64}.  */
+#define BTF_KF_ENUM_UNSIGNED	(0)
+#define BTF_KF_ENUM_SIGNED 	(1 << 0)
+
 /* BTF_KIND_ARRAY is followed by a single struct btf_array.  */
 struct btf_array
 {
@@ -190,6 +194,15 @@ struct btf_var_secinfo
   uint32_t size;	/* Size (in bytes) of variable.  */
 };
 
+/* BTF_KIND_ENUM64 is followed by VLEN struct btf_enum64 entries,
+   which describe the 64 bits enumerators.  */
+struct btf_enum64
+{
+  uint32_t name_off;	/* Offset in string section of enumerator name.  */
+  uint32_t val_lo32;	/* lower 32-bit value for a 64-bit value Enumerator */
+  uint32_t val_hi32;	/* high 32-bit value for a 64-bit value Enumerator */
+};
+
 #ifdef	__cplusplus
 }
 #endif
-- 
2.35.1


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

* Re: [PATCH v2] btf: Add support to BTF_KIND_ENUM64 type
  2022-09-28 21:15 ` [PATCH v2] " Guillermo E. Martinez
@ 2022-09-29 22:35   ` Indu Bhagat
  2022-10-03 14:39     ` Guillermo E. Martinez
  2022-10-15  3:55   ` [PATCH v3] " Guillermo E. Martinez
  1 sibling, 1 reply; 15+ messages in thread
From: Indu Bhagat @ 2022-09-29 22:35 UTC (permalink / raw)
  To: Guillermo E. Martinez, gcc-patches; +Cc: David Faust

On 9/28/22 2:15 PM, Guillermo E. Martinez via Gcc-patches wrote:
> Hello GCC team,
> 
> The following is patch v2 to update BTF/CTF backend supporting
> BTF_KIND_ENUM64 type. Changes from v1:
> 
>    + Fix typo in commit message.
>    + Fix changelog entries.
> 
> Comments will be welcomed and appreciated!,
> 
> Kind regards,
> guillermo
> --
> 

Hi Guillermo,

Thanks for your patch.

Sorry for the delay in reviewing this patch. Please see my comments 
inlined.

Indu

> BTF supports 64-bits enumerators with following encoding:
> 
>    struct btf_type:
>      name_off: 0 or offset to a valid C identifier
>      info.kind_flag: 0 for unsigned, 1 for signed
>      info.kind: BTF_KIND_ENUM64
>      info.vlen: number of enum values
>      size: 1/2/4/8
> 
> The btf_type is followed by info.vlen number of:
> 
>      struct btf_enum64
>      {
>        uint32_t name_off;   /* Offset in string section of enumerator name.  */
>        uint32_t val_lo32;   /* lower 32-bit value for a 64-bit value Enumerator */
>        uint32_t val_hi32;   /* high 32-bit value for a 64-bit value Enumerator */
>      };
> 
> So, a new btf_enum64 structure was added to represent BTF_KIND_ENUM64
> and a new field in ctf_dtdef to represent specific type's properties, in
> the particular case for CTF enums it helps to distinguish when its
> enumerators values are signed or unsigned, later that information is
> used to encode the BTF enum type.
> 
> gcc/ChangeLog:
> 
> 	* btfout.cc (btf_calc_num_vbytes): Compute enumeration size depending of
> 	enumerator type btf_enum{,64}.
> 	(btf_asm_type): Update btf_kflag according to enumerators sign,
> 	using correct BPF type in BTF_KIND_ENUMi{,64}.

Typo : i after ENUM

> 	(btf_asm_enum_const): New argument to represent the size of
> 	the BTF enum type.
> 	* ctfc.cc (ctf_add_enum): Use and initialization of flag field to
> 	CTF_ENUM_F_NONE.
> 	(ctf_add_enumerator): New argument to represent CTF flags,
> 	updating the comment and flag vaue according to enumerators
> 	sing.
> 	* ctfc.h (ctf_dmdef): Update dmd_value to HOST_WIDE_INT to allow
> 	use 32/64 bits enumerators.
> 	(ctf_dtdef): Add flags to to describe specific type's properties.
> 	* dwarf2ctf.cc (gen_ctf_enumeration_type): Update flags field
> 	depending when a signed enumerator value is found.
> 
> include/
> 	* btf.h (btf_enum64): Add new definition and new symbolic
> 	constant to BTF_KIND_ENUM64 and BTF_KF_ENUM_{UN,}SIGNED.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* gcc.dg/debug/btf/btf-enum-1.c: Update testcase, with correct
> 	info.kflags encoding.
> 	* gcc.dg/debug/btf/btf-enum64-1.c: New testcase.
> ---
>   gcc/btfout.cc                                 | 24 ++++++++---
>   gcc/ctfc.cc                                   | 14 ++++---
>   gcc/ctfc.h                                    |  9 +++-
>   gcc/dwarf2ctf.cc                              |  9 +++-
>   gcc/testsuite/gcc.dg/debug/btf/btf-enum-1.c   |  2 +-
>   gcc/testsuite/gcc.dg/debug/btf/btf-enum64-1.c | 41 +++++++++++++++++++
>   include/btf.h                                 | 19 +++++++--
>   7 files changed, 99 insertions(+), 19 deletions(-)
>   create mode 100644 gcc/testsuite/gcc.dg/debug/btf/btf-enum64-1.c
> 
> diff --git a/gcc/btfout.cc b/gcc/btfout.cc
> index 997a33fa089..4b11c867c23 100644
> --- a/gcc/btfout.cc
> +++ b/gcc/btfout.cc
> @@ -223,7 +223,9 @@ btf_calc_num_vbytes (ctf_dtdef_ref dtd)
>         break;
>   
>       case BTF_KIND_ENUM:
> -      vlen_bytes += vlen * sizeof (struct btf_enum);
> +      vlen_bytes += (dtd->dtd_data.ctti_size == 0x8)
> +			? vlen * sizeof (struct btf_enum64)
> +			: vlen * sizeof (struct btf_enum);
>         break;
>   
>       case BTF_KIND_FUNC_PROTO:
> @@ -622,6 +624,15 @@ btf_asm_type (ctf_container_ref ctfc, ctf_dtdef_ref dtd)
>         btf_size_type = 0;
>       }
>   
> + if (btf_kind == BTF_KIND_ENUM)
> +   {
> +     btf_kflag = (dtd->flags & CTF_ENUM_F_ENUMERATORS_SIGNED)
> +		    ? BTF_KF_ENUM_SIGNED
> +		    : BTF_KF_ENUM_UNSIGNED;
> +     if (dtd->dtd_data.ctti_size == 0x8)
> +       btf_kind = BTF_KIND_ENUM64;
> +   }
> +

See below. If you do add a new member in ctf_dmdef instead (as I 
propose), you should ideally iterate over the enumerators 
(dtd->dtd_u.dtu_members) to make sure they are all the same signedness.

>     dw2_asm_output_data (4, dtd->dtd_data.ctti_name, "btt_name");
>     dw2_asm_output_data (4, BTF_TYPE_INFO (btf_kind, btf_kflag, btf_vlen),
>   		       "btt_info: kind=%u, kflag=%u, vlen=%u",
> @@ -634,6 +645,7 @@ btf_asm_type (ctf_container_ref ctfc, ctf_dtdef_ref dtd)
>       case BTF_KIND_UNION:
>       case BTF_KIND_ENUM:
>       case BTF_KIND_DATASEC:
> +    case BTF_KIND_ENUM64:
>         dw2_asm_output_data (4, dtd->dtd_data.ctti_size, "btt_size: %uB",
>   			   dtd->dtd_data.ctti_size);
>         return;
> @@ -707,13 +719,13 @@ btf_asm_sou_member (ctf_container_ref ctfc, ctf_dmdef_t * dmd)
>       }
>   }
>   
> -/* Asm'out an enum constant following a BTF_KIND_ENUM.  */
> +/* Asm'out an enum constant following a BTF_KIND_ENUM{,64}.  */
>   
>   static void
> -btf_asm_enum_const (ctf_dmdef_t * dmd)
> +btf_asm_enum_const (unsigned int size, ctf_dmdef_t * dmd)
>   {
>     dw2_asm_output_data (4, dmd->dmd_name_offset, "bte_name");
> -  dw2_asm_output_data (4, dmd->dmd_value, "bte_value");
> +  dw2_asm_output_data (size, dmd->dmd_value, "bte_value");

I am not sure if this is ok to do like the above. The format 
specification says:

The ``btf_enum64`` encoding:
   * ``name_off``: offset to a valid C identifier
   * ``val_lo32``: lower 32-bit value for a 64-bit value
   * ``val_hi32``: high 32-bit value for a 64-bit value

What if the chosen BPF arch type is big-endian and the const value is 8 
bytes ? Wouldnt such constant values to be written out incorrectly then 
? I think you need to write the lower 32-bits and higher 32-bits 
explicitly to avoid that issue.

>   }
>   
>   /* Asm'out a function parameter description following a BTF_KIND_FUNC_PROTO.  */
> @@ -871,7 +883,7 @@ output_asm_btf_sou_fields (ctf_container_ref ctfc, ctf_dtdef_ref dtd)
>         btf_asm_sou_member (ctfc, dmd);
>   }
>   
> -/* Output all enumerator constants following a BTF_KIND_ENUM.  */
> +/* Output all enumerator constants following a BTF_KIND_ENUM{,64}.  */
>   
>   static void
>   output_asm_btf_enum_list (ctf_container_ref ARG_UNUSED (ctfc),
> @@ -881,7 +893,7 @@ output_asm_btf_enum_list (ctf_container_ref ARG_UNUSED (ctfc),
>   
>     for (dmd = dtd->dtd_u.dtu_members;
>          dmd != NULL; dmd = (ctf_dmdef_t *) ctf_dmd_list_next (dmd))
> -    btf_asm_enum_const (dmd);
> +    btf_asm_enum_const (dtd->dtd_data.ctti_size, dmd);
>   }
>   
>   /* Output all function arguments following a BTF_KIND_FUNC_PROTO.  */
> diff --git a/gcc/ctfc.cc b/gcc/ctfc.cc
> index 9773358a475..253c36b6a0a 100644
> --- a/gcc/ctfc.cc
> +++ b/gcc/ctfc.cc
> @@ -604,6 +604,7 @@ ctf_add_enum (ctf_container_ref ctfc, uint32_t flag, const char * name,
>     gcc_assert (size <= CTF_MAX_SIZE);
>   
>     dtd->dtd_data.ctti_size = size;
> +  dtd->flags = CTF_ENUM_F_NONE;
>   
>     ctfc->ctfc_num_stypes++;
>   
> @@ -612,7 +613,7 @@ ctf_add_enum (ctf_container_ref ctfc, uint32_t flag, const char * name,
>   
>   int
>   ctf_add_enumerator (ctf_container_ref ctfc, ctf_id_t enid, const char * name,
> -		    HOST_WIDE_INT value, dw_die_ref die)
> +		    HOST_WIDE_INT value, uint32_t flags, dw_die_ref die)
>   {
>     ctf_dmdef_t * dmd;
>     uint32_t kind, vlen, root;
> @@ -630,10 +631,12 @@ ctf_add_enumerator (ctf_container_ref ctfc, ctf_id_t enid, const char * name,
>   
>     gcc_assert (kind == CTF_K_ENUM && vlen < CTF_MAX_VLEN);
>   
> -  /* Enum value is of type HOST_WIDE_INT in the compiler, dmd_value is int32_t
> -     on the other hand.  Check bounds and skip adding this enum value if out of
> -     bounds.  */
> -  if ((value > INT_MAX) || (value < INT_MIN))
> +  /* Enum value is of type HOST_WIDE_INT in the compiler, CTF enumerators
> +     values in ctf_enum_t is limited to int32_t, BTF supports signed and
> +     unsigned enumerators values of 32 and 64 bits, for both debug formats
> +     we use ctf_dmdef_t.dmd_value entry of HOST_WIDE_INT type. So check
> +     CTF bounds and skip adding this enum value if out of bounds.  */
> +  if (!btf_debuginfo_p() && ((value > INT_MAX) || (value < INT_MIN)))
>       {
>         /* FIXME - Note this TBD_CTF_REPRESENTATION_LIMIT.  */
>         return (1);
> @@ -649,6 +652,7 @@ ctf_add_enumerator (ctf_container_ref ctfc, ctf_id_t enid, const char * name,
>     dmd->dmd_value = value;
>   
>     dtd->dtd_data.ctti_info = CTF_TYPE_INFO (kind, root, vlen + 1);
> +  dtd->flags |= flags;
>     ctf_dmd_list_append (&dtd->dtd_u.dtu_members, dmd);
>   
>     if ((name != NULL) && strcmp (name, ""))
> diff --git a/gcc/ctfc.h b/gcc/ctfc.h
> index bcf3a43ae1b..a22342b2610 100644
> --- a/gcc/ctfc.h
> +++ b/gcc/ctfc.h
> @@ -125,6 +125,10 @@ typedef struct GTY (()) ctf_itype
>   
>   #define CTF_FUNC_VARARG 0x1
>   
> +/* Enum specific flags.  */
> +#define CTF_ENUM_F_NONE                     (0)
> +#define CTF_ENUM_F_ENUMERATORS_SIGNED       (1 << 0)
> +
>   /* Struct/union/enum member definition for CTF generation.  */
>   
>   typedef struct GTY ((chain_next ("%h.dmd_next"))) ctf_dmdef
> @@ -133,7 +137,7 @@ typedef struct GTY ((chain_next ("%h.dmd_next"))) ctf_dmdef
>     ctf_id_t dmd_type;		/* Type of this member (for sou).  */
>     uint32_t dmd_name_offset;	/* Offset of the name in str table.  */
>     uint64_t dmd_offset;		/* Offset of this member in bits (for sou).  */
> -  int dmd_value;		/* Value of this member (for enum).  */
> +  HOST_WIDE_INT dmd_value;	/* Value of this member (for enum).  */
>     struct ctf_dmdef * dmd_next;	/* A list node.  */
>   } ctf_dmdef_t;
>   

I am wondering if you considered adding a member here instead - 
something like-

bool dmd_value_signed; /* Signedness for the enumerator.  */.

See comment below.

> @@ -162,6 +166,7 @@ struct GTY ((for_user)) ctf_dtdef
>     bool from_global_func; /* Whether this type was added from a global
>   			    function.  */
>     uint32_t linkage;           /* Used in function types.  0=local, 1=global.  */
> +  uint32_t flags;             /* Flags to describe specific type's properties.  */
>     union GTY ((desc ("ctf_dtu_d_union_selector (&%1)")))
>     {
>       /* struct, union, or enum.  */

Instead of carrying this information in ctf_dtdef which is the data 
structure for each type in CTF, how about adding a new member in struct 
ctf_dmdef? The "flags" member is meant for only enum types, and hence it 
will be more appropriate to add to ctf_dmdef as say, dmd_value_signed.

> @@ -429,7 +434,7 @@ extern ctf_id_t ctf_add_sou (ctf_container_ref, uint32_t, const char *,
>   			     uint32_t, size_t, dw_die_ref);
>   
>   extern int ctf_add_enumerator (ctf_container_ref, ctf_id_t, const char *,
> -			       HOST_WIDE_INT, dw_die_ref);
> +			       HOST_WIDE_INT, uint32_t, dw_die_ref);
>   extern int ctf_add_member_offset (ctf_container_ref, dw_die_ref, const char *,
>   				  ctf_id_t, uint64_t);
>   extern int ctf_add_function_arg (ctf_container_ref, dw_die_ref,
> diff --git a/gcc/dwarf2ctf.cc b/gcc/dwarf2ctf.cc
> index 397100004c2..0ef96dd48fd 100644
> --- a/gcc/dwarf2ctf.cc
> +++ b/gcc/dwarf2ctf.cc
> @@ -772,6 +772,7 @@ gen_ctf_enumeration_type (ctf_container_ref ctfc, dw_die_ref enumeration)
>   	  const char *enumerator_name;
>   	  dw_attr_node *enumerator_value;
>   	  HOST_WIDE_INT value_wide_int;
> +	  uint32_t flags = 0;
>   
>   	  c = dw_get_die_sib (c);
>   
> @@ -785,10 +786,14 @@ gen_ctf_enumeration_type (ctf_container_ref ctfc, dw_die_ref enumeration)
>   		  == dw_val_class_unsigned_const_implicit))
>   	    value_wide_int = AT_unsigned (enumerator_value);
>   	  else
> -	    value_wide_int = AT_int (enumerator_value);
> +	    {
> +	      value_wide_int = AT_int (enumerator_value);
> +	      flags |= CTF_ENUM_F_ENUMERATORS_SIGNED;
> +	    }
>   
>   	  ctf_add_enumerator (ctfc, enumeration_type_id,
> -			      enumerator_name, value_wide_int, enumeration);
> +			      enumerator_name, value_wide_int,
> +			      flags, enumeration);
>   	}
>         while (c != dw_get_die_child (enumeration));
>     }
> diff --git a/gcc/testsuite/gcc.dg/debug/btf/btf-enum-1.c b/gcc/testsuite/gcc.dg/debug/btf/btf-enum-1.c
> index 728493b0804..7e940529f1b 100644
> --- a/gcc/testsuite/gcc.dg/debug/btf/btf-enum-1.c
> +++ b/gcc/testsuite/gcc.dg/debug/btf/btf-enum-1.c
> @@ -4,7 +4,7 @@
>   /* { dg-options "-O0 -gbtf -dA" } */
>   
>   /* { dg-final { scan-assembler-times "\[\t \]0x6000004\[\t \]+\[^\n\]*btt_info" 1 } } */
> -/* { dg-final { scan-assembler-times "\[\t \]0x6000003\[\t \]+\[^\n\]*btt_info" 1 } } */
> +/* { dg-final { scan-assembler-times "\[\t \]0x86000003\[\t \]+\[^\n\]*btt_info" 1 } } */
>   /* { dg-final { scan-assembler-times "ascii \"QAD.0\"\[\t \]+\[^\n\]*btf_string" 1 } } */
>   /* { dg-final { scan-assembler-times "ascii \"QED.0\"\[\t \]+\[^\n\]*btf_string" 1 } } */
>   /* { dg-final { scan-assembler-times "ascii \"QOD.0\"\[\t \]+\[^\n\]*btf_string" 1 } } */
> diff --git a/gcc/testsuite/gcc.dg/debug/btf/btf-enum64-1.c b/gcc/testsuite/gcc.dg/debug/btf/btf-enum64-1.c
> new file mode 100644
> index 00000000000..da103842807
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/debug/btf/btf-enum64-1.c
> @@ -0,0 +1,41 @@
> +/* Test BTF generation for 64 bits enums.  */
> +
> +/* { dg-do compile } */
> +/* { dg-options "-O0 -gbtf -dA" } */
> +
> +/* { dg-final { scan-assembler-times "\[\t \].size\[\t \]myenum1,\[\t \]8" 1 } } */
> +/* { dg-final { scan-assembler-times "\[\t \].size\[\t \]myenum2,\[\t \]8" 1 } } */
> +/* { dg-final { scan-assembler-times "\[\t \].size\[\t \]myenum3,\[\t \]8" 1 } } */
> +/* { dg-final { scan-assembler-times "\[\t \]0x13000003\[\t \]+\[^\n\]*btt_info" 2 } } */
> +/* { dg-final { scan-assembler-times "\[\t \]0x93000003\[\t \]+\[^\n\]*btt_info" 1 } } */
> +/* { dg-final { scan-assembler-times "ascii \"B1.0\"\[\t \]+\[^\n\]*btf_string" 1 } } */
> +/* { dg-final { scan-assembler-times "ascii \"B2.0\"\[\t \]+\[^\n\]*btf_string" 1 } } */
> +/* { dg-final { scan-assembler-times "ascii \"B3.0\"\[\t \]+\[^\n\]*btf_string" 1 } } */
> +/* { dg-final { scan-assembler-times "ascii \"C1.0\"\[\t \]+\[^\n\]*btf_string" 1 } } */
> +/* { dg-final { scan-assembler-times "ascii \"C2.0\"\[\t \]+\[^\n\]*btf_string" 1 } } */
> +/* { dg-final { scan-assembler-times "ascii \"C3.0\"\[\t \]+\[^\n\]*btf_string" 1 } } */
> +/* { dg-final { scan-assembler-times "ascii \"D1.0\"\[\t \]+\[^\n\]*btf_string" 1 } } */
> +/* { dg-final { scan-assembler-times "ascii \"D2.0\"\[\t \]+\[^\n\]*btf_string" 1 } } */
> +/* { dg-final { scan-assembler-times "ascii \"D3.0\"\[\t \]+\[^\n\]*btf_string" 1 } } */
> +/* { dg-final { scan-assembler-times "bte_value" 9 } } */
> +
> +enum default_enum
> +{
> +  B1 = 0xffffffffaa,
> +  B2 = 0xbbbbbbbb,
> +  B3 = 0xaabbccdd,
> +} myenum1 = B1;
> +
> +enum explicit_unsigned
> +{
> +  C1 = 0xffffffffbbUL,
> +  C2 = 0xbbbbbbbb,
> +  C3 = 0xaabbccdd,
> +} myenum2 = C1;
> +
> +enum signed64
> +{
> +  D1 = 0xffffffffaa,
> +  D2 = 0xbbbbbbbb,
> +  D3 = -0x1,
> +} myenum3 = D1;

I wonder if its meaningul to add such an enum64 test with 
-mlittle-endian and -mbig-endian to keep the order of val_lo32 and 
val_hi32 tested.

> diff --git a/include/btf.h b/include/btf.h
> index 78b551ced23..eba67f9d599 100644
> --- a/include/btf.h
> +++ b/include/btf.h
> @@ -109,7 +109,8 @@ struct btf_type
>   #define BTF_KIND_VAR		14	/* Variable.  */
>   #define BTF_KIND_DATASEC	15	/* Section such as .bss or .data.  */
>   #define BTF_KIND_FLOAT		16	/* Floating point.  */
> -#define BTF_KIND_MAX		BTF_KIND_FLOAT
> +#define BTF_KIND_ENUM64 	19	/* Enumeration up to 64 bits.  */
> +#define BTF_KIND_MAX		BTF_KIND_ENUM64
>   #define NR_BTF_KINDS		(BTF_KIND_MAX + 1)
>   
>   /* For some BTF_KINDs, struct btf_type is immediately followed by
> @@ -130,14 +131,17 @@ struct btf_type
>   #define BTF_INT_BOOL	(1 << 2)
>   
>   /* BTF_KIND_ENUM is followed by VLEN struct btf_enum entries,
> -   which describe the enumerators. Note that BTF currently only
> -   supports signed 32-bit enumerator values.  */
> +   which describe the enumerators. */
>   struct btf_enum
>   {
>     uint32_t name_off;	/* Offset in string section of enumerator name.  */
>     int32_t  val;		/* Enumerator value.  */
>   };
>   
> +/* BTF_KF_ENUM_ holds the flags for kflags in BTF_KIND_ENUM{,64}.  */
> +#define BTF_KF_ENUM_UNSIGNED	(0)
> +#define BTF_KF_ENUM_SIGNED 	(1 << 0)
> +
>   /* BTF_KIND_ARRAY is followed by a single struct btf_array.  */
>   struct btf_array
>   {
> @@ -190,6 +194,15 @@ struct btf_var_secinfo
>     uint32_t size;	/* Size (in bytes) of variable.  */
>   };
>   
> +/* BTF_KIND_ENUM64 is followed by VLEN struct btf_enum64 entries,
> +   which describe the 64 bits enumerators.  */
> +struct btf_enum64
> +{
> +  uint32_t name_off;	/* Offset in string section of enumerator name.  */
> +  uint32_t val_lo32;	/* lower 32-bit value for a 64-bit value Enumerator */
> +  uint32_t val_hi32;	/* high 32-bit value for a 64-bit value Enumerator */
> +};
> +
>   #ifdef	__cplusplus
>   }
>   #endif
> 


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

* Re: [PATCH v2] btf: Add support to BTF_KIND_ENUM64 type
  2022-09-29 22:35   ` Indu Bhagat
@ 2022-10-03 14:39     ` Guillermo E. Martinez
  2022-10-11 18:55       ` Indu Bhagat
  0 siblings, 1 reply; 15+ messages in thread
From: Guillermo E. Martinez @ 2022-10-03 14:39 UTC (permalink / raw)
  To: Indu Bhagat, gcc-patches; +Cc: David Faust



On 9/29/22 17:35, Indu Bhagat wrote:
> On 9/28/22 2:15 PM, Guillermo E. Martinez via Gcc-patches wrote:
>> Hello GCC team,
>>
>> The following is patch v2 to update BTF/CTF backend supporting
>> BTF_KIND_ENUM64 type. Changes from v1:
>>
>>    + Fix typo in commit message.
>>    + Fix changelog entries.
>>
>> Comments will be welcomed and appreciated!,
>>
>> Kind regards,
>> guillermo
>> -- 
>>
> 
> Hi Guillermo,
> 

Hi Indu,

> Thanks for your patch.
> 
> Sorry for the delay in reviewing this patch. Please see my comments inlined.
> 

No worries, thanks so much for your comments!. They were really useful.

> Indu
> 
>> BTF supports 64-bits enumerators with following encoding:
>>
>>    struct btf_type:
>>      name_off: 0 or offset to a valid C identifier
>>      info.kind_flag: 0 for unsigned, 1 for signed
>>      info.kind: BTF_KIND_ENUM64
>>      info.vlen: number of enum values
>>      size: 1/2/4/8
>>
>> The btf_type is followed by info.vlen number of:
>>
>>      struct btf_enum64
>>      {
>>        uint32_t name_off;   /* Offset in string section of enumerator name.  */
>>        uint32_t val_lo32;   /* lower 32-bit value for a 64-bit value Enumerator */
>>        uint32_t val_hi32;   /* high 32-bit value for a 64-bit value Enumerator */
>>      };
>>
>> So, a new btf_enum64 structure was added to represent BTF_KIND_ENUM64
>> and a new field in ctf_dtdef to represent specific type's properties, in
>> the particular case for CTF enums it helps to distinguish when its
>> enumerators values are signed or unsigned, later that information is
>> used to encode the BTF enum type.
>>
>> gcc/ChangeLog:
>>
>>     * btfout.cc (btf_calc_num_vbytes): Compute enumeration size depending of
>>     enumerator type btf_enum{,64}.
>>     (btf_asm_type): Update btf_kflag according to enumerators sign,
>>     using correct BPF type in BTF_KIND_ENUMi{,64}.
> 
> Typo : i after ENUM
> 

Fixed in v3.

>>     (btf_asm_enum_const): New argument to represent the size of
>>     the BTF enum type.
>>     * ctfc.cc (ctf_add_enum): Use and initialization of flag field to
>>     CTF_ENUM_F_NONE.
>>     (ctf_add_enumerator): New argument to represent CTF flags,
>>     updating the comment and flag vaue according to enumerators
>>     sing.
>>     * ctfc.h (ctf_dmdef): Update dmd_value to HOST_WIDE_INT to allow
>>     use 32/64 bits enumerators.
>>     (ctf_dtdef): Add flags to to describe specific type's properties.
>>     * dwarf2ctf.cc (gen_ctf_enumeration_type): Update flags field
>>     depending when a signed enumerator value is found.
>>
>> include/
>>     * btf.h (btf_enum64): Add new definition and new symbolic
>>     constant to BTF_KIND_ENUM64 and BTF_KF_ENUM_{UN,}SIGNED.
>>
>> gcc/testsuite/ChangeLog:
>>
>>     * gcc.dg/debug/btf/btf-enum-1.c: Update testcase, with correct
>>     info.kflags encoding.
>>     * gcc.dg/debug/btf/btf-enum64-1.c: New testcase.
>> ---
>>   gcc/btfout.cc                                 | 24 ++++++++---
>>   gcc/ctfc.cc                                   | 14 ++++---
>>   gcc/ctfc.h                                    |  9 +++-
>>   gcc/dwarf2ctf.cc                              |  9 +++-
>>   gcc/testsuite/gcc.dg/debug/btf/btf-enum-1.c   |  2 +-
>>   gcc/testsuite/gcc.dg/debug/btf/btf-enum64-1.c | 41 +++++++++++++++++++
>>   include/btf.h                                 | 19 +++++++--
>>   7 files changed, 99 insertions(+), 19 deletions(-)
>>   create mode 100644 gcc/testsuite/gcc.dg/debug/btf/btf-enum64-1.c
>>
>> diff --git a/gcc/btfout.cc b/gcc/btfout.cc
>> index 997a33fa089..4b11c867c23 100644
>> --- a/gcc/btfout.cc
>> +++ b/gcc/btfout.cc
>> @@ -223,7 +223,9 @@ btf_calc_num_vbytes (ctf_dtdef_ref dtd)
>>         break;
>>       case BTF_KIND_ENUM:
>> -      vlen_bytes += vlen * sizeof (struct btf_enum);
>> +      vlen_bytes += (dtd->dtd_data.ctti_size == 0x8)
>> +            ? vlen * sizeof (struct btf_enum64)
>> +            : vlen * sizeof (struct btf_enum);
>>         break;
>>       case BTF_KIND_FUNC_PROTO:
>> @@ -622,6 +624,15 @@ btf_asm_type (ctf_container_ref ctfc, ctf_dtdef_ref dtd)
>>         btf_size_type = 0;
>>       }
>> + if (btf_kind == BTF_KIND_ENUM)
>> +   {
>> +     btf_kflag = (dtd->flags & CTF_ENUM_F_ENUMERATORS_SIGNED)
>> +            ? BTF_KF_ENUM_SIGNED
>> +            : BTF_KF_ENUM_UNSIGNED;
>> +     if (dtd->dtd_data.ctti_size == 0x8)
>> +       btf_kind = BTF_KIND_ENUM64;
>> +   }
>> +
> 
> See below. If you do add a new member in ctf_dmdef instead (as I propose), you should ideally iterate over the enumerators (dtd->dtd_u.dtu_members) to make sure they are all the same signedness.
> 

Correct, struct `ctf_dmdef' stores enumerator's information. However
in BTF spec ``info.kind_flag`` is not a property per enumerator, but
enumeration type property instead, so I decided to add in `ctf_dtdef'
a `flag' field to store the signeedness information for an enumeration
type, then in `gen_ctf_enumeration_type' when it builds the enumerators
entries,`ctf_dtdef.flag' is updated to `CTF_ENUM_F_ENUMERATORS_SIGNED'
when  it found a signed enumerator value meaning that enumeration type
should be consider a signed type. i.e, I detect signeedness when
enumeration type is being building.

Of course, I can add `dmd_value_signed' field in `ctf_dmdef' but I need
to iterate over all `ctf_dmdef.dmd_value_signed' (in `btf_asm_type' I guess)
to properly set ``info.kind_flag`` if it's a signed enumeration type.

>>     dw2_asm_output_data (4, dtd->dtd_data.ctti_name, "btt_name");
>>     dw2_asm_output_data (4, BTF_TYPE_INFO (btf_kind, btf_kflag, btf_vlen),
>>                  "btt_info: kind=%u, kflag=%u, vlen=%u",
>> @@ -634,6 +645,7 @@ btf_asm_type (ctf_container_ref ctfc, ctf_dtdef_ref dtd)
>>       case BTF_KIND_UNION:
>>       case BTF_KIND_ENUM:
>>       case BTF_KIND_DATASEC:
>> +    case BTF_KIND_ENUM64:
>>         dw2_asm_output_data (4, dtd->dtd_data.ctti_size, "btt_size: %uB",
>>                  dtd->dtd_data.ctti_size);
>>         return;
>> @@ -707,13 +719,13 @@ btf_asm_sou_member (ctf_container_ref ctfc, ctf_dmdef_t * dmd)
>>       }
>>   }
>> -/* Asm'out an enum constant following a BTF_KIND_ENUM.  */
>> +/* Asm'out an enum constant following a BTF_KIND_ENUM{,64}.  */
>>   static void
>> -btf_asm_enum_const (ctf_dmdef_t * dmd)
>> +btf_asm_enum_const (unsigned int size, ctf_dmdef_t * dmd)
>>   {
>>     dw2_asm_output_data (4, dmd->dmd_name_offset, "bte_name");
>> -  dw2_asm_output_data (4, dmd->dmd_value, "bte_value");
>> +  dw2_asm_output_data (size, dmd->dmd_value, "bte_value");
> 
> I am not sure if this is ok to do like the above. The format specification says:
> 
No, It is not right.

> The ``btf_enum64`` encoding:
>    * ``name_off``: offset to a valid C identifier
>    * ``val_lo32``: lower 32-bit value for a 64-bit value
>    * ``val_hi32``: high 32-bit value for a 64-bit value
> 
> What if the chosen BPF arch type is big-endian and the const value is 8 bytes ? Wouldnt such constant values to be written out incorrectly then ? I think you need to write the lower 32-bits and higher 32-bits explicitly to avoid that issue.
> 

Totally agree. Fixed in v3.

>>   }
>>   /* Asm'out a function parameter description following a BTF_KIND_FUNC_PROTO.  */
>> @@ -871,7 +883,7 @@ output_asm_btf_sou_fields (ctf_container_ref ctfc, ctf_dtdef_ref dtd)
>>         btf_asm_sou_member (ctfc, dmd);
>>   }
>> -/* Output all enumerator constants following a BTF_KIND_ENUM.  */
>> +/* Output all enumerator constants following a BTF_KIND_ENUM{,64}.  */
>>   static void
>>   output_asm_btf_enum_list (ctf_container_ref ARG_UNUSED (ctfc),
>> @@ -881,7 +893,7 @@ output_asm_btf_enum_list (ctf_container_ref ARG_UNUSED (ctfc),
>>     for (dmd = dtd->dtd_u.dtu_members;
>>          dmd != NULL; dmd = (ctf_dmdef_t *) ctf_dmd_list_next (dmd))
>> -    btf_asm_enum_const (dmd);
>> +    btf_asm_enum_const (dtd->dtd_data.ctti_size, dmd);
>>   }
>>   /* Output all function arguments following a BTF_KIND_FUNC_PROTO.  */
>> diff --git a/gcc/ctfc.cc b/gcc/ctfc.cc
>> index 9773358a475..253c36b6a0a 100644
>> --- a/gcc/ctfc.cc
>> +++ b/gcc/ctfc.cc
>> @@ -604,6 +604,7 @@ ctf_add_enum (ctf_container_ref ctfc, uint32_t flag, const char * name,
>>     gcc_assert (size <= CTF_MAX_SIZE);
>>     dtd->dtd_data.ctti_size = size;
>> +  dtd->flags = CTF_ENUM_F_NONE;
>>     ctfc->ctfc_num_stypes++;
>> @@ -612,7 +613,7 @@ ctf_add_enum (ctf_container_ref ctfc, uint32_t flag, const char * name,
>>   int
>>   ctf_add_enumerator (ctf_container_ref ctfc, ctf_id_t enid, const char * name,
>> -            HOST_WIDE_INT value, dw_die_ref die)
>> +            HOST_WIDE_INT value, uint32_t flags, dw_die_ref die)
>>   {
>>     ctf_dmdef_t * dmd;
>>     uint32_t kind, vlen, root;
>> @@ -630,10 +631,12 @@ ctf_add_enumerator (ctf_container_ref ctfc, ctf_id_t enid, const char * name,
>>     gcc_assert (kind == CTF_K_ENUM && vlen < CTF_MAX_VLEN);
>> -  /* Enum value is of type HOST_WIDE_INT in the compiler, dmd_value is int32_t
>> -     on the other hand.  Check bounds and skip adding this enum value if out of
>> -     bounds.  */
>> -  if ((value > INT_MAX) || (value < INT_MIN))
>> +  /* Enum value is of type HOST_WIDE_INT in the compiler, CTF enumerators
>> +     values in ctf_enum_t is limited to int32_t, BTF supports signed and
>> +     unsigned enumerators values of 32 and 64 bits, for both debug formats
>> +     we use ctf_dmdef_t.dmd_value entry of HOST_WIDE_INT type. So check
>> +     CTF bounds and skip adding this enum value if out of bounds.  */
>> +  if (!btf_debuginfo_p() && ((value > INT_MAX) || (value < INT_MIN)))
>>       {
>>         /* FIXME - Note this TBD_CTF_REPRESENTATION_LIMIT.  */
>>         return (1);
>> @@ -649,6 +652,7 @@ ctf_add_enumerator (ctf_container_ref ctfc, ctf_id_t enid, const char * name,
>>     dmd->dmd_value = value;
>>     dtd->dtd_data.ctti_info = CTF_TYPE_INFO (kind, root, vlen + 1);
>> +  dtd->flags |= flags;
>>     ctf_dmd_list_append (&dtd->dtd_u.dtu_members, dmd);
>>     if ((name != NULL) && strcmp (name, ""))
>> diff --git a/gcc/ctfc.h b/gcc/ctfc.h
>> index bcf3a43ae1b..a22342b2610 100644
>> --- a/gcc/ctfc.h
>> +++ b/gcc/ctfc.h
>> @@ -125,6 +125,10 @@ typedef struct GTY (()) ctf_itype
>>   #define CTF_FUNC_VARARG 0x1
>> +/* Enum specific flags.  */
>> +#define CTF_ENUM_F_NONE                     (0)
>> +#define CTF_ENUM_F_ENUMERATORS_SIGNED       (1 << 0)
>> +
>>   /* Struct/union/enum member definition for CTF generation.  */
>>   typedef struct GTY ((chain_next ("%h.dmd_next"))) ctf_dmdef
>> @@ -133,7 +137,7 @@ typedef struct GTY ((chain_next ("%h.dmd_next"))) ctf_dmdef
>>     ctf_id_t dmd_type;        /* Type of this member (for sou).  */
>>     uint32_t dmd_name_offset;    /* Offset of the name in str table.  */
>>     uint64_t dmd_offset;        /* Offset of this member in bits (for sou).  */
>> -  int dmd_value;        /* Value of this member (for enum).  */
>> +  HOST_WIDE_INT dmd_value;    /* Value of this member (for enum).  */
>>     struct ctf_dmdef * dmd_next;    /* A list node.  */
>>   } ctf_dmdef_t;
> 
> I am wondering if you considered adding a member here instead - something like-
> 
> bool dmd_value_signed; /* Signedness for the enumerator.  */.
> 
> See comment below.
> 
>> @@ -162,6 +166,7 @@ struct GTY ((for_user)) ctf_dtdef
>>     bool from_global_func; /* Whether this type was added from a global
>>                   function.  */
>>     uint32_t linkage;           /* Used in function types.  0=local, 1=global.  */
>> +  uint32_t flags;             /* Flags to describe specific type's properties.  */
>>     union GTY ((desc ("ctf_dtu_d_union_selector (&%1)")))
>>     {
>>       /* struct, union, or enum.  */
> 
> Instead of carrying this information in ctf_dtdef which is the data structure for each type in CTF, how about adding a new member in struct ctf_dmdef? The "flags" member is meant for only enum types, and hence it will be more appropriate to add to ctf_dmdef as say, dmd_value_signed.
> 

Yes, `ctf_dtdef' is structure for each type in CTF (including enumeration),
and `ctf_dmdef' keeps information for enumerator, not for the enumeration type.

>> @@ -429,7 +434,7 @@ extern ctf_id_t ctf_add_sou (ctf_container_ref, uint32_t, const char *,
>>                    uint32_t, size_t, dw_die_ref);
>>   extern int ctf_add_enumerator (ctf_container_ref, ctf_id_t, const char *,
>> -                   HOST_WIDE_INT, dw_die_ref);
>> +                   HOST_WIDE_INT, uint32_t, dw_die_ref);
>>   extern int ctf_add_member_offset (ctf_container_ref, dw_die_ref, const char *,
>>                     ctf_id_t, uint64_t);
>>   extern int ctf_add_function_arg (ctf_container_ref, dw_die_ref,
>> diff --git a/gcc/dwarf2ctf.cc b/gcc/dwarf2ctf.cc
>> index 397100004c2..0ef96dd48fd 100644
>> --- a/gcc/dwarf2ctf.cc
>> +++ b/gcc/dwarf2ctf.cc
>> @@ -772,6 +772,7 @@ gen_ctf_enumeration_type (ctf_container_ref ctfc, dw_die_ref enumeration)
>>         const char *enumerator_name;
>>         dw_attr_node *enumerator_value;
>>         HOST_WIDE_INT value_wide_int;
>> +      uint32_t flags = 0;
>>         c = dw_get_die_sib (c);
>> @@ -785,10 +786,14 @@ gen_ctf_enumeration_type (ctf_container_ref ctfc, dw_die_ref enumeration)
>>             == dw_val_class_unsigned_const_implicit))
>>           value_wide_int = AT_unsigned (enumerator_value);
>>         else
>> -        value_wide_int = AT_int (enumerator_value);
>> +        {
>> +          value_wide_int = AT_int (enumerator_value);
>> +          flags |= CTF_ENUM_F_ENUMERATORS_SIGNED;
>> +        }
>>         ctf_add_enumerator (ctfc, enumeration_type_id,
>> -                  enumerator_name, value_wide_int, enumeration);
>> +                  enumerator_name, value_wide_int,
>> +                  flags, enumeration);
>>       }
>>         while (c != dw_get_die_child (enumeration));
>>     }
>> diff --git a/gcc/testsuite/gcc.dg/debug/btf/btf-enum-1.c b/gcc/testsuite/gcc.dg/debug/btf/btf-enum-1.c
>> index 728493b0804..7e940529f1b 100644
>> --- a/gcc/testsuite/gcc.dg/debug/btf/btf-enum-1.c
>> +++ b/gcc/testsuite/gcc.dg/debug/btf/btf-enum-1.c
>> @@ -4,7 +4,7 @@
>>   /* { dg-options "-O0 -gbtf -dA" } */
>>   /* { dg-final { scan-assembler-times "\[\t \]0x6000004\[\t \]+\[^\n\]*btt_info" 1 } } */
>> -/* { dg-final { scan-assembler-times "\[\t \]0x6000003\[\t \]+\[^\n\]*btt_info" 1 } } */
>> +/* { dg-final { scan-assembler-times "\[\t \]0x86000003\[\t \]+\[^\n\]*btt_info" 1 } } */
>>   /* { dg-final { scan-assembler-times "ascii \"QAD.0\"\[\t \]+\[^\n\]*btf_string" 1 } } */
>>   /* { dg-final { scan-assembler-times "ascii \"QED.0\"\[\t \]+\[^\n\]*btf_string" 1 } } */
>>   /* { dg-final { scan-assembler-times "ascii \"QOD.0\"\[\t \]+\[^\n\]*btf_string" 1 } } */
>> diff --git a/gcc/testsuite/gcc.dg/debug/btf/btf-enum64-1.c b/gcc/testsuite/gcc.dg/debug/btf/btf-enum64-1.c
>> new file mode 100644
>> index 00000000000..da103842807
>> --- /dev/null
>> +++ b/gcc/testsuite/gcc.dg/debug/btf/btf-enum64-1.c
>> @@ -0,0 +1,41 @@
>> +/* Test BTF generation for 64 bits enums.  */
>> +
>> +/* { dg-do compile } */
>> +/* { dg-options "-O0 -gbtf -dA" } */
>> +
>> +/* { dg-final { scan-assembler-times "\[\t \].size\[\t \]myenum1,\[\t \]8" 1 } } */
>> +/* { dg-final { scan-assembler-times "\[\t \].size\[\t \]myenum2,\[\t \]8" 1 } } */
>> +/* { dg-final { scan-assembler-times "\[\t \].size\[\t \]myenum3,\[\t \]8" 1 } } */
>> +/* { dg-final { scan-assembler-times "\[\t \]0x13000003\[\t \]+\[^\n\]*btt_info" 2 } } */
>> +/* { dg-final { scan-assembler-times "\[\t \]0x93000003\[\t \]+\[^\n\]*btt_info" 1 } } */
>> +/* { dg-final { scan-assembler-times "ascii \"B1.0\"\[\t \]+\[^\n\]*btf_string" 1 } } */
>> +/* { dg-final { scan-assembler-times "ascii \"B2.0\"\[\t \]+\[^\n\]*btf_string" 1 } } */
>> +/* { dg-final { scan-assembler-times "ascii \"B3.0\"\[\t \]+\[^\n\]*btf_string" 1 } } */
>> +/* { dg-final { scan-assembler-times "ascii \"C1.0\"\[\t \]+\[^\n\]*btf_string" 1 } } */
>> +/* { dg-final { scan-assembler-times "ascii \"C2.0\"\[\t \]+\[^\n\]*btf_string" 1 } } */
>> +/* { dg-final { scan-assembler-times "ascii \"C3.0\"\[\t \]+\[^\n\]*btf_string" 1 } } */
>> +/* { dg-final { scan-assembler-times "ascii \"D1.0\"\[\t \]+\[^\n\]*btf_string" 1 } } */
>> +/* { dg-final { scan-assembler-times "ascii \"D2.0\"\[\t \]+\[^\n\]*btf_string" 1 } } */
>> +/* { dg-final { scan-assembler-times "ascii \"D3.0\"\[\t \]+\[^\n\]*btf_string" 1 } } */
>> +/* { dg-final { scan-assembler-times "bte_value" 9 } } */
>> +
>> +enum default_enum
>> +{
>> +  B1 = 0xffffffffaa,
>> +  B2 = 0xbbbbbbbb,
>> +  B3 = 0xaabbccdd,
>> +} myenum1 = B1;
>> +
>> +enum explicit_unsigned
>> +{
>> +  C1 = 0xffffffffbbUL,
>> +  C2 = 0xbbbbbbbb,
>> +  C3 = 0xaabbccdd,
>> +} myenum2 = C1;
>> +
>> +enum signed64
>> +{
>> +  D1 = 0xffffffffaa,
>> +  D2 = 0xbbbbbbbb,
>> +  D3 = -0x1,
>> +} myenum3 = D1;
> 
> I wonder if its meaningul to add such an enum64 test with -mlittle-endian and -mbig-endian to keep the order of val_lo32 and val_hi32 tested.
> 

Very meaningful!. I'll add use cases in v3 for both endianess.

>> diff --git a/include/btf.h b/include/btf.h
>> index 78b551ced23..eba67f9d599 100644
>> --- a/include/btf.h
>> +++ b/include/btf.h
>> @@ -109,7 +109,8 @@ struct btf_type
>>   #define BTF_KIND_VAR        14    /* Variable.  */
>>   #define BTF_KIND_DATASEC    15    /* Section such as .bss or .data.  */
>>   #define BTF_KIND_FLOAT        16    /* Floating point.  */
>> -#define BTF_KIND_MAX        BTF_KIND_FLOAT
>> +#define BTF_KIND_ENUM64     19    /* Enumeration up to 64 bits.  */
>> +#define BTF_KIND_MAX        BTF_KIND_ENUM64
>>   #define NR_BTF_KINDS        (BTF_KIND_MAX + 1)
>>   /* For some BTF_KINDs, struct btf_type is immediately followed by
>> @@ -130,14 +131,17 @@ struct btf_type
>>   #define BTF_INT_BOOL    (1 << 2)
>>   /* BTF_KIND_ENUM is followed by VLEN struct btf_enum entries,
>> -   which describe the enumerators. Note that BTF currently only
>> -   supports signed 32-bit enumerator values.  */
>> +   which describe the enumerators. */
>>   struct btf_enum
>>   {
>>     uint32_t name_off;    /* Offset in string section of enumerator name.  */
>>     int32_t  val;        /* Enumerator value.  */
>>   };
>> +/* BTF_KF_ENUM_ holds the flags for kflags in BTF_KIND_ENUM{,64}.  */
>> +#define BTF_KF_ENUM_UNSIGNED    (0)
>> +#define BTF_KF_ENUM_SIGNED     (1 << 0)
>> +
>>   /* BTF_KIND_ARRAY is followed by a single struct btf_array.  */
>>   struct btf_array
>>   {
>> @@ -190,6 +194,15 @@ struct btf_var_secinfo
>>     uint32_t size;    /* Size (in bytes) of variable.  */
>>   };
>> +/* BTF_KIND_ENUM64 is followed by VLEN struct btf_enum64 entries,
>> +   which describe the 64 bits enumerators.  */
>> +struct btf_enum64
>> +{
>> +  uint32_t name_off;    /* Offset in string section of enumerator name.  */
>> +  uint32_t val_lo32;    /* lower 32-bit value for a 64-bit value Enumerator */
>> +  uint32_t val_hi32;    /* high 32-bit value for a 64-bit value Enumerator */
>> +};
>> +
>>   #ifdef    __cplusplus
>>   }
>>   #endif
>>
> 

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

* Re: [PATCH v2] btf: Add support to BTF_KIND_ENUM64 type
  2022-10-03 14:39     ` Guillermo E. Martinez
@ 2022-10-11 18:55       ` Indu Bhagat
  2022-10-15  3:45         ` Guillermo E. Martinez
  0 siblings, 1 reply; 15+ messages in thread
From: Indu Bhagat @ 2022-10-11 18:55 UTC (permalink / raw)
  To: Guillermo E. Martinez, gcc-patches

Hi Guillermo,

On 10/3/22 7:39 AM, Guillermo E. Martinez via Gcc-patches wrote:
>>> diff --git a/gcc/ctfc.cc b/gcc/ctfc.cc
>>> index 9773358a475..253c36b6a0a 100644
>>> --- a/gcc/ctfc.cc
>>> +++ b/gcc/ctfc.cc
>>> @@ -604,6 +604,7 @@ ctf_add_enum (ctf_container_ref ctfc, uint32_t 
>>> flag, const char * name,
>>>     gcc_assert (size <= CTF_MAX_SIZE);
>>>     dtd->dtd_data.ctti_size = size;
>>> +  dtd->flags = CTF_ENUM_F_NONE;
>>>     ctfc->ctfc_num_stypes++;
>>> @@ -612,7 +613,7 @@ ctf_add_enum (ctf_container_ref ctfc, uint32_t 
>>> flag, const char * name,
>>>   int
>>>   ctf_add_enumerator (ctf_container_ref ctfc, ctf_id_t enid, const 
>>> char * name,
>>> -            HOST_WIDE_INT value, dw_die_ref die)
>>> +            HOST_WIDE_INT value, uint32_t flags, dw_die_ref die)
>>>   {
>>>     ctf_dmdef_t * dmd;
>>>     uint32_t kind, vlen, root;
>>> @@ -630,10 +631,12 @@ ctf_add_enumerator (ctf_container_ref ctfc, 
>>> ctf_id_t enid, const char * name,
>>>     gcc_assert (kind == CTF_K_ENUM && vlen < CTF_MAX_VLEN);
>>> -  /* Enum value is of type HOST_WIDE_INT in the compiler, dmd_value 
>>> is int32_t
>>> -     on the other hand.  Check bounds and skip adding this enum 
>>> value if out of
>>> -     bounds.  */
>>> -  if ((value > INT_MAX) || (value < INT_MIN))
>>> +  /* Enum value is of type HOST_WIDE_INT in the compiler, CTF 
>>> enumerators
>>> +     values in ctf_enum_t is limited to int32_t, BTF supports signed 
>>> and
>>> +     unsigned enumerators values of 32 and 64 bits, for both debug 
>>> formats
>>> +     we use ctf_dmdef_t.dmd_value entry of HOST_WIDE_INT type. So check
>>> +     CTF bounds and skip adding this enum value if out of bounds.  */
>>> +  if (!btf_debuginfo_p() && ((value > INT_MAX) || (value < INT_MIN)))
>>>       {
>>>         /* FIXME - Note this TBD_CTF_REPRESENTATION_LIMIT.  */
>>>         return (1);
>>> @@ -649,6 +652,7 @@ ctf_add_enumerator (ctf_container_ref ctfc, 
>>> ctf_id_t enid, const char * name,
>>>     dmd->dmd_value = value;
>>>     dtd->dtd_data.ctti_info = CTF_TYPE_INFO (kind, root, vlen + 1);
>>> +  dtd->flags |= flags;
>>>     ctf_dmd_list_append (&dtd->dtd_u.dtu_members, dmd);
>>>     if ((name != NULL) && strcmp (name, ""))
>>> diff --git a/gcc/ctfc.h b/gcc/ctfc.h
>>> index bcf3a43ae1b..a22342b2610 100644
>>> --- a/gcc/ctfc.h
>>> +++ b/gcc/ctfc.h
>>> @@ -125,6 +125,10 @@ typedef struct GTY (()) ctf_itype
>>>   #define CTF_FUNC_VARARG 0x1
>>> +/* Enum specific flags.  */
>>> +#define CTF_ENUM_F_NONE                     (0)
>>> +#define CTF_ENUM_F_ENUMERATORS_SIGNED       (1 << 0)
>>> +
>>>   /* Struct/union/enum member definition for CTF generation.  */
>>>   typedef struct GTY ((chain_next ("%h.dmd_next"))) ctf_dmdef
>>> @@ -133,7 +137,7 @@ typedef struct GTY ((chain_next ("%h.dmd_next"))) 
>>> ctf_dmdef
>>>     ctf_id_t dmd_type;        /* Type of this member (for sou).  */
>>>     uint32_t dmd_name_offset;    /* Offset of the name in str table.  */
>>>     uint64_t dmd_offset;        /* Offset of this member in bits (for 
>>> sou).  */
>>> -  int dmd_value;        /* Value of this member (for enum).  */
>>> +  HOST_WIDE_INT dmd_value;    /* Value of this member (for enum).  */
>>>     struct ctf_dmdef * dmd_next;    /* A list node.  */
>>>   } ctf_dmdef_t;
>>
>> I am wondering if you considered adding a member here instead - 
>> something like-
>>
>> bool dmd_value_signed; /* Signedness for the enumerator.  */.
>>
>> See comment below.
>>
>>> @@ -162,6 +166,7 @@ struct GTY ((for_user)) ctf_dtdef
>>>     bool from_global_func; /* Whether this type was added from a global
>>>                   function.  */
>>>     uint32_t linkage;           /* Used in function types.  0=local, 
>>> 1=global.  */
>>> +  uint32_t flags;             /* Flags to describe specific type's 
>>> properties.  */
>>>     union GTY ((desc ("ctf_dtu_d_union_selector (&%1)")))
>>>     {
>>>       /* struct, union, or enum.  */
>>
>> Instead of carrying this information in ctf_dtdef which is the data 
>> structure for each type in CTF, how about adding a new member in 
>> struct ctf_dmdef? The "flags" member is meant for only enum types, and 
>> hence it will be more appropriate to add to ctf_dmdef as say, 
>> dmd_value_signed.
>>
> 
> Yes, `ctf_dtdef' is structure for each type in CTF (including enumeration),
> and `ctf_dmdef' keeps information for enumerator, not for the 
> enumeration type.

Yes, please scrap my earlier suggestion of adding to ctf_dmdef_t.

What do you think about adding something like 'dtd_enum_signedness' to 
ctf_dtdef, instead of uint32_t 'flags'; with two possible values of 0 
(unsigned) and 1 (signed).

I believe your intention of using the latter is to conserve some memory 
in the long run (by reusing the flags field for other types in future if 
need be)? I do, however, prefer an explicit member like 
dtd_enum_signedness at this time. My reasoning for keeping it explicit 
is that it helps code be more readable/maintainable.

Thanks for your patience,
Indu

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

* Re: [PATCH v2] btf: Add support to BTF_KIND_ENUM64 type
  2022-10-11 18:55       ` Indu Bhagat
@ 2022-10-15  3:45         ` Guillermo E. Martinez
  0 siblings, 0 replies; 15+ messages in thread
From: Guillermo E. Martinez @ 2022-10-15  3:45 UTC (permalink / raw)
  To: Indu Bhagat, gcc-patches



On 10/11/22 13:55, Indu Bhagat wrote:
> Hi Guillermo,
> 

Hi Indu,

> On 10/3/22 7:39 AM, Guillermo E. Martinez via Gcc-patches wrote:
>>>> diff --git a/gcc/ctfc.cc b/gcc/ctfc.cc
>>>> index 9773358a475..253c36b6a0a 100644
>>>> --- a/gcc/ctfc.cc
>>>> +++ b/gcc/ctfc.cc
>>>> @@ -604,6 +604,7 @@ ctf_add_enum (ctf_container_ref ctfc, uint32_t flag, const char * name,
>>>>     gcc_assert (size <= CTF_MAX_SIZE);
>>>>     dtd->dtd_data.ctti_size = size;
>>>> +  dtd->flags = CTF_ENUM_F_NONE;
>>>>     ctfc->ctfc_num_stypes++;
>>>> @@ -612,7 +613,7 @@ ctf_add_enum (ctf_container_ref ctfc, uint32_t flag, const char * name,
>>>>   int
>>>>   ctf_add_enumerator (ctf_container_ref ctfc, ctf_id_t enid, const char * name,
>>>> -            HOST_WIDE_INT value, dw_die_ref die)
>>>> +            HOST_WIDE_INT value, uint32_t flags, dw_die_ref die)
>>>>   {
>>>>     ctf_dmdef_t * dmd;
>>>>     uint32_t kind, vlen, root;
>>>> @@ -630,10 +631,12 @@ ctf_add_enumerator (ctf_container_ref ctfc, ctf_id_t enid, const char * name,
>>>>     gcc_assert (kind == CTF_K_ENUM && vlen < CTF_MAX_VLEN);
>>>> -  /* Enum value is of type HOST_WIDE_INT in the compiler, dmd_value is int32_t
>>>> -     on the other hand.  Check bounds and skip adding this enum value if out of
>>>> -     bounds.  */
>>>> -  if ((value > INT_MAX) || (value < INT_MIN))
>>>> +  /* Enum value is of type HOST_WIDE_INT in the compiler, CTF enumerators
>>>> +     values in ctf_enum_t is limited to int32_t, BTF supports signed and
>>>> +     unsigned enumerators values of 32 and 64 bits, for both debug formats
>>>> +     we use ctf_dmdef_t.dmd_value entry of HOST_WIDE_INT type. So check
>>>> +     CTF bounds and skip adding this enum value if out of bounds.  */
>>>> +  if (!btf_debuginfo_p() && ((value > INT_MAX) || (value < INT_MIN)))
>>>>       {
>>>>         /* FIXME - Note this TBD_CTF_REPRESENTATION_LIMIT.  */
>>>>         return (1);
>>>> @@ -649,6 +652,7 @@ ctf_add_enumerator (ctf_container_ref ctfc, ctf_id_t enid, const char * name,
>>>>     dmd->dmd_value = value;
>>>>     dtd->dtd_data.ctti_info = CTF_TYPE_INFO (kind, root, vlen + 1);
>>>> +  dtd->flags |= flags;
>>>>     ctf_dmd_list_append (&dtd->dtd_u.dtu_members, dmd);
>>>>     if ((name != NULL) && strcmp (name, ""))
>>>> diff --git a/gcc/ctfc.h b/gcc/ctfc.h
>>>> index bcf3a43ae1b..a22342b2610 100644
>>>> --- a/gcc/ctfc.h
>>>> +++ b/gcc/ctfc.h
>>>> @@ -125,6 +125,10 @@ typedef struct GTY (()) ctf_itype
>>>>   #define CTF_FUNC_VARARG 0x1
>>>> +/* Enum specific flags.  */
>>>> +#define CTF_ENUM_F_NONE                     (0)
>>>> +#define CTF_ENUM_F_ENUMERATORS_SIGNED       (1 << 0)
>>>> +
>>>>   /* Struct/union/enum member definition for CTF generation.  */
>>>>   typedef struct GTY ((chain_next ("%h.dmd_next"))) ctf_dmdef
>>>> @@ -133,7 +137,7 @@ typedef struct GTY ((chain_next ("%h.dmd_next"))) ctf_dmdef
>>>>     ctf_id_t dmd_type;        /* Type of this member (for sou).  */
>>>>     uint32_t dmd_name_offset;    /* Offset of the name in str table.  */
>>>>     uint64_t dmd_offset;        /* Offset of this member in bits (for sou).  */
>>>> -  int dmd_value;        /* Value of this member (for enum).  */
>>>> +  HOST_WIDE_INT dmd_value;    /* Value of this member (for enum).  */
>>>>     struct ctf_dmdef * dmd_next;    /* A list node.  */
>>>>   } ctf_dmdef_t;
>>>
>>> I am wondering if you considered adding a member here instead - something like-
>>>
>>> bool dmd_value_signed; /* Signedness for the enumerator.  */.
>>>
>>> See comment below.
>>>
>>>> @@ -162,6 +166,7 @@ struct GTY ((for_user)) ctf_dtdef
>>>>     bool from_global_func; /* Whether this type was added from a global
>>>>                   function.  */
>>>>     uint32_t linkage;           /* Used in function types.  0=local, 1=global.  */
>>>> +  uint32_t flags;             /* Flags to describe specific type's properties.  */
>>>>     union GTY ((desc ("ctf_dtu_d_union_selector (&%1)")))
>>>>     {
>>>>       /* struct, union, or enum.  */
>>>
>>> Instead of carrying this information in ctf_dtdef which is the data structure for each type in CTF, how about adding a new member in struct ctf_dmdef? The "flags" member is meant for only enum types, and hence it will be more appropriate to add to ctf_dmdef as say, dmd_value_signed.
>>>
>>
>> Yes, `ctf_dtdef' is structure for each type in CTF (including enumeration),
>> and `ctf_dmdef' keeps information for enumerator, not for the enumeration type.
> 
> Yes, please scrap my earlier suggestion of adding to ctf_dmdef_t.
> 
> What do you think about adding something like 'dtd_enum_signedness' to ctf_dtdef, instead of uint32_t 'flags'; with two possible values of 0 (unsigned) and 1 (signed).
> 

OK. I'll use a bool variable field named `dtd_enum_unsiged' like to ENUMERAL_TYPE
does storing signedness in `.base.u.bits.unsigned_flag', later this information
is used to set the DW_AT_encoding attribute to DW_ATE_{unsigned,signed}, finally
with this information we can set the dtd_enum_unsiged variable in
`gen_ctf_enumeration_type', so no need to iterate over enumerators constant
to figure out signedness of the enumeration type. But, please let me know
your thoughts in patchv3.


> I believe your intention of using the latter is to conserve some memory in the long run (by reusing the flags field for other types in future if need be)? I do, however, prefer an explicit member like dtd_enum_signedness at this time. My reasoning for keeping it explicit is that it helps code be more readable/maintainable.
> 

Sure, I will do so.

> Thanks for your patience,

No, thanks to you for your comments!

> Indu

guillermo

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

* [PATCH v3] btf: Add support to BTF_KIND_ENUM64 type
  2022-09-28 21:15 ` [PATCH v2] " Guillermo E. Martinez
  2022-09-29 22:35   ` Indu Bhagat
@ 2022-10-15  3:55   ` Guillermo E. Martinez
  2022-10-18 21:40     ` Indu Bhagat
  2022-10-20  2:05     ` [PATCH v4] " Guillermo E. Martinez
  1 sibling, 2 replies; 15+ messages in thread
From: Guillermo E. Martinez @ 2022-10-15  3:55 UTC (permalink / raw)
  To: gcc-patches; +Cc: indu.bhagat, david.faust, Guillermo E. Martinez

Hello,

The following is patch v3 to update BTF/CTF backend supporting
BTF_KIND_ENUM64 type. Changes from v2:

  + Add a new `dtd_enum_unsigned' field in `ctf_dtdef' to indicate
    signedness of the enum type.
  + Fix endianness for representing BTF enum 64-bits enumerators.
  + Add {little,big}-endian testcases.

Comments will be welcomed and appreciated!,

Kind regards,
guillermo

--

BTF supports 64-bits enumerators with following encoding:

  struct btf_type:
    name_off: 0 or offset to a valid C identifier
    info.kind_flag: 0 for unsigned, 1 for signed
    info.kind: BTF_KIND_ENUM64
    info.vlen: number of enum values
    size: 1/2/4/8

The btf_type is followed by info.vlen number of:

    struct btf_enum64
    {
      uint32_t name_off;   /* Offset in string section of enumerator name.  */
      uint32_t val_lo32;   /* lower 32-bit value for a 64-bit value Enumerator */
      uint32_t val_hi32;   /* high 32-bit value for a 64-bit value Enumerator */
    };

So, a new btf_enum64 structure was added to represent BTF_KIND_ENUM64
and a new field dtd_enum_unsigned in ctf_dtdef structure to distinguish
when CTF enum is a signed or unsigned type, later that information is
used to encode the BTF enum type.

gcc/ChangeLog:

	* btfout.cc (btf_calc_num_vbytes): Compute enumeration size depending of
	enumerator type btf_enum{,64}.
	(btf_asm_type): Update btf_kflag according to enumeration type sign
	using dtd_enum_unsigned field for both:  BTF_KIND_ENUM{,64}.
	(btf_asm_enum_const): New argument to represent the size of
	the BTF enum type, writing the enumerator constant value for
	32 bits, if it's 64 bits then explicitly writes lower 32-bits
	value and higher 32-bits value.
	(output_asm_btf_enum_list): Add enumeration size argument.
	* ctfc.cc (ctf_add_enum): New argument to represent CTF enum
	basic information.
	(ctf_add_generic): Use of ei_{name. size, unsigned} to build the
	dtd structure containing enumeration information.
	(ctf_add_enumerator): Update comment mention support for BTF
	enumeration in 64-bits.
	* ctfc.h (ctf_dmdef): Update dmd_value to HOST_WIDE_INT to allow
	use 32/64 bits enumerators.
	(ctf_enum_binfo): New type to represent CTF basic enum type
	information.
	(ctf_dtdef): New field to describe enum signedness.
	* dwarf2ctf.cc (gen_ctf_enumeration_type): Use of ctf_enum_binfo
	type to pass information to ctf_add_enum to build the enum type.

include/
	* btf.h (btf_enum64): Add new definition and new symbolic
	constant to BTF_KIND_ENUM64 and BTF_KF_ENUM_{UN,}SIGNED.

gcc/testsuite/ChangeLog:

	* gcc.dg/debug/btf/btf-enum-1.c: Update testcase, with correct
	info.kflags encoding.
	* gcc.dg/debug/btf/btf-enum64-be-1.c: New testcase.
	* gcc.dg/debug/btf/btf-enum64-le-1.c: New testcase.
---
 gcc/btfout.cc                                 | 30 ++++++++++---
 gcc/ctfc.cc                                   | 22 +++++-----
 gcc/ctfc.h                                    | 15 +++++--
 gcc/dwarf2ctf.cc                              |  8 +++-
 gcc/testsuite/gcc.dg/debug/btf/btf-enum-1.c   |  2 +-
 .../gcc.dg/debug/btf/btf-enum64-be-1.c        | 44 +++++++++++++++++++
 .../gcc.dg/debug/btf/btf-enum64-le-1.c        | 44 +++++++++++++++++++
 include/btf.h                                 | 19 ++++++--
 8 files changed, 160 insertions(+), 24 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/debug/btf/btf-enum64-be-1.c
 create mode 100644 gcc/testsuite/gcc.dg/debug/btf/btf-enum64-le-1.c

diff --git a/gcc/btfout.cc b/gcc/btfout.cc
index 997a33fa089..aef9fd70a28 100644
--- a/gcc/btfout.cc
+++ b/gcc/btfout.cc
@@ -223,7 +223,9 @@ btf_calc_num_vbytes (ctf_dtdef_ref dtd)
       break;
 
     case BTF_KIND_ENUM:
-      vlen_bytes += vlen * sizeof (struct btf_enum);
+      vlen_bytes += (dtd->dtd_data.ctti_size == 0x8)
+			? vlen * sizeof (struct btf_enum64)
+			: vlen * sizeof (struct btf_enum);
       break;
 
     case BTF_KIND_FUNC_PROTO:
@@ -622,6 +624,15 @@ btf_asm_type (ctf_container_ref ctfc, ctf_dtdef_ref dtd)
       btf_size_type = 0;
     }
 
+  if (btf_kind == BTF_KIND_ENUM)
+    {
+      btf_kflag = dtd->dtd_enum_unsigned
+		    ? BTF_KF_ENUM_UNSIGNED
+		    : BTF_KF_ENUM_SIGNED;
+      if (dtd->dtd_data.ctti_size == 0x8)
+	btf_kind = BTF_KIND_ENUM64;
+   }
+
   dw2_asm_output_data (4, dtd->dtd_data.ctti_name, "btt_name");
   dw2_asm_output_data (4, BTF_TYPE_INFO (btf_kind, btf_kflag, btf_vlen),
 		       "btt_info: kind=%u, kflag=%u, vlen=%u",
@@ -634,6 +645,7 @@ btf_asm_type (ctf_container_ref ctfc, ctf_dtdef_ref dtd)
     case BTF_KIND_UNION:
     case BTF_KIND_ENUM:
     case BTF_KIND_DATASEC:
+    case BTF_KIND_ENUM64:
       dw2_asm_output_data (4, dtd->dtd_data.ctti_size, "btt_size: %uB",
 			   dtd->dtd_data.ctti_size);
       return;
@@ -707,13 +719,19 @@ btf_asm_sou_member (ctf_container_ref ctfc, ctf_dmdef_t * dmd)
     }
 }
 
-/* Asm'out an enum constant following a BTF_KIND_ENUM.  */
+/* Asm'out an enum constant following a BTF_KIND_ENUM{,64}.  */
 
 static void
-btf_asm_enum_const (ctf_dmdef_t * dmd)
+btf_asm_enum_const (unsigned int size, ctf_dmdef_t * dmd)
 {
   dw2_asm_output_data (4, dmd->dmd_name_offset, "bte_name");
-  dw2_asm_output_data (4, dmd->dmd_value, "bte_value");
+  if (size == 4)
+    dw2_asm_output_data (size, dmd->dmd_value, "bte_value");
+  else
+    {
+      dw2_asm_output_data (4, dmd->dmd_value & 0xffffffff, "bte_value_lo32");
+      dw2_asm_output_data (4, (dmd->dmd_value >> 32) & 0xffffffff, "bte_value_hi32");
+    }
 }
 
 /* Asm'out a function parameter description following a BTF_KIND_FUNC_PROTO.  */
@@ -871,7 +889,7 @@ output_asm_btf_sou_fields (ctf_container_ref ctfc, ctf_dtdef_ref dtd)
       btf_asm_sou_member (ctfc, dmd);
 }
 
-/* Output all enumerator constants following a BTF_KIND_ENUM.  */
+/* Output all enumerator constants following a BTF_KIND_ENUM{,64}.  */
 
 static void
 output_asm_btf_enum_list (ctf_container_ref ARG_UNUSED (ctfc),
@@ -881,7 +899,7 @@ output_asm_btf_enum_list (ctf_container_ref ARG_UNUSED (ctfc),
 
   for (dmd = dtd->dtd_u.dtu_members;
        dmd != NULL; dmd = (ctf_dmdef_t *) ctf_dmd_list_next (dmd))
-    btf_asm_enum_const (dmd);
+    btf_asm_enum_const (dtd->dtd_data.ctti_size, dmd);
 }
 
 /* Output all function arguments following a BTF_KIND_FUNC_PROTO.  */
diff --git a/gcc/ctfc.cc b/gcc/ctfc.cc
index 9773358a475..80322eea91a 100644
--- a/gcc/ctfc.cc
+++ b/gcc/ctfc.cc
@@ -576,8 +576,8 @@ ctf_add_array (ctf_container_ref ctfc, uint32_t flag, const ctf_arinfo_t * arp,
 }
 
 ctf_id_t
-ctf_add_enum (ctf_container_ref ctfc, uint32_t flag, const char * name,
-	      HOST_WIDE_INT size, dw_die_ref die)
+ctf_add_enum (ctf_container_ref ctfc, uint32_t flag,
+	      const ctf_enum_binfo_t *ei, dw_die_ref die)
 {
   ctf_dtdef_ref dtd;
   ctf_id_t type;
@@ -595,16 +595,16 @@ ctf_add_enum (ctf_container_ref ctfc, uint32_t flag, const char * name,
 	= CTF_TYPE_INFO (CTF_K_FORWARD, CTF_ADD_NONROOT, 0);
     }
 
-  type = ctf_add_generic (ctfc, flag, name, &dtd, die);
+  type = ctf_add_generic (ctfc, flag, ei->ei_name, &dtd, die);
 
   dtd->dtd_data.ctti_info = CTF_TYPE_INFO (CTF_K_ENUM, flag, 0);
 
   /* Size in bytes should always fit, of course.
      TBD WARN - warn instead?  */
-  gcc_assert (size <= CTF_MAX_SIZE);
-
-  dtd->dtd_data.ctti_size = size;
+  gcc_assert (ei->ei_size <= CTF_MAX_SIZE);
 
+  dtd->dtd_data.ctti_size = ei->ei_size;
+  dtd->dtd_enum_unsigned = ei->ei_unsigned;
   ctfc->ctfc_num_stypes++;
 
   return type;
@@ -630,10 +630,12 @@ ctf_add_enumerator (ctf_container_ref ctfc, ctf_id_t enid, const char * name,
 
   gcc_assert (kind == CTF_K_ENUM && vlen < CTF_MAX_VLEN);
 
-  /* Enum value is of type HOST_WIDE_INT in the compiler, dmd_value is int32_t
-     on the other hand.  Check bounds and skip adding this enum value if out of
-     bounds.  */
-  if ((value > INT_MAX) || (value < INT_MIN))
+  /* Enum value is of type HOST_WIDE_INT in the compiler, CTF enumerators
+     values in ctf_enum_t is limited to int32_t, BTF supports signed and
+     unsigned enumerators values of 32 and 64 bits, for both debug formats
+     we use ctf_dmdef_t.dmd_value entry of HOST_WIDE_INT type. So check
+     CTF bounds and skip adding this enum value if out of bounds.  */
+  if (!btf_debuginfo_p() && ((value > INT_MAX) || (value < INT_MIN)))
     {
       /* FIXME - Note this TBD_CTF_REPRESENTATION_LIMIT.  */
       return (1);
diff --git a/gcc/ctfc.h b/gcc/ctfc.h
index bcf3a43ae1b..d3f50454385 100644
--- a/gcc/ctfc.h
+++ b/gcc/ctfc.h
@@ -125,6 +125,14 @@ typedef struct GTY (()) ctf_itype
 
 #define CTF_FUNC_VARARG 0x1
 
+/* Basic enum information to build ctf_dmdef_t type.  */
+typedef struct ctf_enum_binfo
+{
+  const char * ei_name;
+  unsigned int ei_size;	    /* Size in bytes.  */
+  bool ei_unsigned;
+} ctf_enum_binfo_t;
+
 /* Struct/union/enum member definition for CTF generation.  */
 
 typedef struct GTY ((chain_next ("%h.dmd_next"))) ctf_dmdef
@@ -133,7 +141,7 @@ typedef struct GTY ((chain_next ("%h.dmd_next"))) ctf_dmdef
   ctf_id_t dmd_type;		/* Type of this member (for sou).  */
   uint32_t dmd_name_offset;	/* Offset of the name in str table.  */
   uint64_t dmd_offset;		/* Offset of this member in bits (for sou).  */
-  int dmd_value;		/* Value of this member (for enum).  */
+  HOST_WIDE_INT dmd_value;	/* Value of this member (for enum).  */
   struct ctf_dmdef * dmd_next;	/* A list node.  */
 } ctf_dmdef_t;
 
@@ -162,6 +170,7 @@ struct GTY ((for_user)) ctf_dtdef
   bool from_global_func; /* Whether this type was added from a global
 			    function.  */
   uint32_t linkage;           /* Used in function types.  0=local, 1=global.  */
+  bool dtd_enum_unsigned;     /* Enum signedness.  */
   union GTY ((desc ("ctf_dtu_d_union_selector (&%1)")))
   {
     /* struct, union, or enum.  */
@@ -405,8 +414,8 @@ extern const char * ctf_add_string (ctf_container_ref, const char *,
 
 extern ctf_id_t ctf_add_reftype (ctf_container_ref, uint32_t, ctf_id_t,
 				 uint32_t, dw_die_ref);
-extern ctf_id_t ctf_add_enum (ctf_container_ref, uint32_t, const char *,
-			      HOST_WIDE_INT, dw_die_ref);
+extern ctf_id_t ctf_add_enum (ctf_container_ref, uint32_t,
+			      const ctf_enum_binfo_t *, dw_die_ref);
 extern ctf_id_t ctf_add_slice (ctf_container_ref, uint32_t, ctf_id_t,
 			       uint32_t, uint32_t, dw_die_ref);
 extern ctf_id_t ctf_add_float (ctf_container_ref, uint32_t, const char *,
diff --git a/gcc/dwarf2ctf.cc b/gcc/dwarf2ctf.cc
index 397100004c2..56dfcadd3fb 100644
--- a/gcc/dwarf2ctf.cc
+++ b/gcc/dwarf2ctf.cc
@@ -736,6 +736,7 @@ gen_ctf_enumeration_type (ctf_container_ref ctfc, dw_die_ref enumeration)
 {
   const char *enum_name = get_AT_string (enumeration, DW_AT_name);
   unsigned int bit_size = ctf_die_bitsize (enumeration);
+  unsigned int signedness = get_AT_unsigned (enumeration, DW_AT_encoding);
   int declaration_p = get_AT_flag (enumeration, DW_AT_declaration);
 
   ctf_id_t enumeration_type_id;
@@ -757,9 +758,14 @@ gen_ctf_enumeration_type (ctf_container_ref ctfc, dw_die_ref enumeration)
       bit_size = ctf_die_bitsize (type);
     }
 
+  ctf_enum_binfo_t ei;
+  ei.ei_name = enum_name;
+  ei.ei_size = bit_size / 8;
+  ei.ei_unsigned = (signedness == DW_ATE_unsigned);
+
   /* Generate a CTF type for the enumeration.  */
   enumeration_type_id = ctf_add_enum (ctfc, CTF_ADD_ROOT,
-				      enum_name, bit_size / 8, enumeration);
+				      &ei, enumeration);
 
   /* Process the enumerators.  */
   {
diff --git a/gcc/testsuite/gcc.dg/debug/btf/btf-enum-1.c b/gcc/testsuite/gcc.dg/debug/btf/btf-enum-1.c
index 728493b0804..7e940529f1b 100644
--- a/gcc/testsuite/gcc.dg/debug/btf/btf-enum-1.c
+++ b/gcc/testsuite/gcc.dg/debug/btf/btf-enum-1.c
@@ -4,7 +4,7 @@
 /* { dg-options "-O0 -gbtf -dA" } */
 
 /* { dg-final { scan-assembler-times "\[\t \]0x6000004\[\t \]+\[^\n\]*btt_info" 1 } } */
-/* { dg-final { scan-assembler-times "\[\t \]0x6000003\[\t \]+\[^\n\]*btt_info" 1 } } */
+/* { dg-final { scan-assembler-times "\[\t \]0x86000003\[\t \]+\[^\n\]*btt_info" 1 } } */
 /* { dg-final { scan-assembler-times "ascii \"QAD.0\"\[\t \]+\[^\n\]*btf_string" 1 } } */
 /* { dg-final { scan-assembler-times "ascii \"QED.0\"\[\t \]+\[^\n\]*btf_string" 1 } } */
 /* { dg-final { scan-assembler-times "ascii \"QOD.0\"\[\t \]+\[^\n\]*btf_string" 1 } } */
diff --git a/gcc/testsuite/gcc.dg/debug/btf/btf-enum64-be-1.c b/gcc/testsuite/gcc.dg/debug/btf/btf-enum64-be-1.c
new file mode 100644
index 00000000000..40d44556e16
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/debug/btf/btf-enum64-be-1.c
@@ -0,0 +1,44 @@
+/* Test BTF generation for 64 bits enums.  */
+
+/* { dg-do compile } */
+/* { dg-options "-O0 -gbtf -dA -mbig-endian" } */
+
+/* { dg-final { scan-assembler-times "\[\t \].size\[\t \]myenum1,\[\t \]8" 1 } } */
+/* { dg-final { scan-assembler-times "\[\t \].size\[\t \]myenum2,\[\t \]8" 1 } } */
+/* { dg-final { scan-assembler-times "\[\t \].size\[\t \]myenum3,\[\t \]8" 1 } } */
+/* { dg-final { scan-assembler-times "\[\t \]0x13000003\[\t \]+\[^\n\]*btt_info" 2 } } */
+/* { dg-final { scan-assembler-times "\[\t \]0x93000003\[\t \]+\[^\n\]*btt_info" 1 } } */
+/* { dg-final { scan-assembler-times "\[\t \]0xffffffaa\[\t \]+\[^\n\]*bte_value_lo32" 2 } } */
+/* { dg-final { scan-assembler-times "\[\t \]0xff\[\t \]+\[^\n\]*bte_value_hi32" 3 } } */
+/* { dg-final { scan-assembler-times "ascii \"B1.0\"\[\t \]+\[^\n\]*btf_string" 1 } } */
+/* { dg-final { scan-assembler-times "ascii \"B2.0\"\[\t \]+\[^\n\]*btf_string" 1 } } */
+/* { dg-final { scan-assembler-times "ascii \"B3.0\"\[\t \]+\[^\n\]*btf_string" 1 } } */
+/* { dg-final { scan-assembler-times "ascii \"C1.0\"\[\t \]+\[^\n\]*btf_string" 1 } } */
+/* { dg-final { scan-assembler-times "ascii \"C2.0\"\[\t \]+\[^\n\]*btf_string" 1 } } */
+/* { dg-final { scan-assembler-times "ascii \"C3.0\"\[\t \]+\[^\n\]*btf_string" 1 } } */
+/* { dg-final { scan-assembler-times "ascii \"D1.0\"\[\t \]+\[^\n\]*btf_string" 1 } } */
+/* { dg-final { scan-assembler-times "ascii \"D2.0\"\[\t \]+\[^\n\]*btf_string" 1 } } */
+/* { dg-final { scan-assembler-times "ascii \"D3.0\"\[\t \]+\[^\n\]*btf_string" 1 } } */
+/* { dg-final { scan-assembler-times "bte_value_lo32" 9 } } */
+/* { dg-final { scan-assembler-times "bte_value_hi32" 9 } } */
+
+enum default_enum
+{
+  B1 = 0xffffffffaa,
+  B2 = 0xbbbbbbbb,
+  B3 = 0xaabbccdd,
+} myenum1 = B1;
+
+enum explicit_unsigned
+{
+  C1 = 0xffffffffbbUL,
+  C2 = 0xbbbbbbbb,
+  C3 = 0xaabbccdd,
+} myenum2 = C1;
+
+enum signed64
+{
+  D1 = 0xffffffffaa,
+  D2 = 0xbbbbbbbb,
+  D3 = -0x1,
+} myenum3 = D1;
diff --git a/gcc/testsuite/gcc.dg/debug/btf/btf-enum64-le-1.c b/gcc/testsuite/gcc.dg/debug/btf/btf-enum64-le-1.c
new file mode 100644
index 00000000000..873a50ea174
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/debug/btf/btf-enum64-le-1.c
@@ -0,0 +1,44 @@
+/* Test BTF generation for 64 bits enums.  */
+
+/* { dg-do compile } */
+/* { dg-options "-O0 -gbtf -dA -mlittle-endian" } */
+
+/* { dg-final { scan-assembler-times "\[\t \].size\[\t \]myenum1,\[\t \]8" 1 } } */
+/* { dg-final { scan-assembler-times "\[\t \].size\[\t \]myenum2,\[\t \]8" 1 } } */
+/* { dg-final { scan-assembler-times "\[\t \].size\[\t \]myenum3,\[\t \]8" 1 } } */
+/* { dg-final { scan-assembler-times "\[\t \]0x13000003\[\t \]+\[^\n\]*btt_info" 2 } } */
+/* { dg-final { scan-assembler-times "\[\t \]0x93000003\[\t \]+\[^\n\]*btt_info" 1 } } */
+/* { dg-final { scan-assembler-times "\[\t \]0xffffffaa\[\t \]+\[^\n\]*bte_value_lo32" 2 } } */
+/* { dg-final { scan-assembler-times "\[\t \]0xff\[\t \]+\[^\n\]*bte_value_hi32" 3 } } */
+/* { dg-final { scan-assembler-times "ascii \"B1.0\"\[\t \]+\[^\n\]*btf_string" 1 } } */
+/* { dg-final { scan-assembler-times "ascii \"B2.0\"\[\t \]+\[^\n\]*btf_string" 1 } } */
+/* { dg-final { scan-assembler-times "ascii \"B3.0\"\[\t \]+\[^\n\]*btf_string" 1 } } */
+/* { dg-final { scan-assembler-times "ascii \"C1.0\"\[\t \]+\[^\n\]*btf_string" 1 } } */
+/* { dg-final { scan-assembler-times "ascii \"C2.0\"\[\t \]+\[^\n\]*btf_string" 1 } } */
+/* { dg-final { scan-assembler-times "ascii \"C3.0\"\[\t \]+\[^\n\]*btf_string" 1 } } */
+/* { dg-final { scan-assembler-times "ascii \"D1.0\"\[\t \]+\[^\n\]*btf_string" 1 } } */
+/* { dg-final { scan-assembler-times "ascii \"D2.0\"\[\t \]+\[^\n\]*btf_string" 1 } } */
+/* { dg-final { scan-assembler-times "ascii \"D3.0\"\[\t \]+\[^\n\]*btf_string" 1 } } */
+/* { dg-final { scan-assembler-times "bte_value_lo32" 9 } } */
+/* { dg-final { scan-assembler-times "bte_value_hi32" 9 } } */
+
+enum default_enum
+{
+  B1 = 0xffffffffaa,
+  B2 = 0xbbbbbbbb,
+  B3 = 0xaabbccdd,
+} myenum1 = B1;
+
+enum explicit_unsigned
+{
+  C1 = 0xffffffffbbUL,
+  C2 = 0xbbbbbbbb,
+  C3 = 0xaabbccdd,
+} myenum2 = C1;
+
+enum signed64
+{
+  D1 = 0xffffffffaa,
+  D2 = 0xbbbbbbbb,
+  D3 = -0x1,
+} myenum3 = D1;
diff --git a/include/btf.h b/include/btf.h
index 78b551ced23..eba67f9d599 100644
--- a/include/btf.h
+++ b/include/btf.h
@@ -109,7 +109,8 @@ struct btf_type
 #define BTF_KIND_VAR		14	/* Variable.  */
 #define BTF_KIND_DATASEC	15	/* Section such as .bss or .data.  */
 #define BTF_KIND_FLOAT		16	/* Floating point.  */
-#define BTF_KIND_MAX		BTF_KIND_FLOAT
+#define BTF_KIND_ENUM64 	19	/* Enumeration up to 64 bits.  */
+#define BTF_KIND_MAX		BTF_KIND_ENUM64
 #define NR_BTF_KINDS		(BTF_KIND_MAX + 1)
 
 /* For some BTF_KINDs, struct btf_type is immediately followed by
@@ -130,14 +131,17 @@ struct btf_type
 #define BTF_INT_BOOL	(1 << 2)
 
 /* BTF_KIND_ENUM is followed by VLEN struct btf_enum entries,
-   which describe the enumerators. Note that BTF currently only
-   supports signed 32-bit enumerator values.  */
+   which describe the enumerators. */
 struct btf_enum
 {
   uint32_t name_off;	/* Offset in string section of enumerator name.  */
   int32_t  val;		/* Enumerator value.  */
 };
 
+/* BTF_KF_ENUM_ holds the flags for kflags in BTF_KIND_ENUM{,64}.  */
+#define BTF_KF_ENUM_UNSIGNED	(0)
+#define BTF_KF_ENUM_SIGNED 	(1 << 0)
+
 /* BTF_KIND_ARRAY is followed by a single struct btf_array.  */
 struct btf_array
 {
@@ -190,6 +194,15 @@ struct btf_var_secinfo
   uint32_t size;	/* Size (in bytes) of variable.  */
 };
 
+/* BTF_KIND_ENUM64 is followed by VLEN struct btf_enum64 entries,
+   which describe the 64 bits enumerators.  */
+struct btf_enum64
+{
+  uint32_t name_off;	/* Offset in string section of enumerator name.  */
+  uint32_t val_lo32;	/* lower 32-bit value for a 64-bit value Enumerator */
+  uint32_t val_hi32;	/* high 32-bit value for a 64-bit value Enumerator */
+};
+
 #ifdef	__cplusplus
 }
 #endif
-- 
2.35.1


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

* Re: [PATCH v3] btf: Add support to BTF_KIND_ENUM64 type
  2022-10-15  3:55   ` [PATCH v3] " Guillermo E. Martinez
@ 2022-10-18 21:40     ` Indu Bhagat
  2022-10-18 21:48       ` Guillermo E. Martinez
  2022-10-20  2:05     ` [PATCH v4] " Guillermo E. Martinez
  1 sibling, 1 reply; 15+ messages in thread
From: Indu Bhagat @ 2022-10-18 21:40 UTC (permalink / raw)
  To: Guillermo E. Martinez, gcc-patches; +Cc: david.faust

Hi Guillermo,

On 10/14/22 8:55 PM, Guillermo E. Martinez wrote:
> Hello,
> 
> The following is patch v3 to update BTF/CTF backend supporting
> BTF_KIND_ENUM64 type. Changes from v2:
> 
>    + Add a new `dtd_enum_unsigned' field in `ctf_dtdef' to indicate
>      signedness of the enum type.
>    + Fix endianness for representing BTF enum 64-bits enumerators.
>    + Add {little,big}-endian testcases.
> 
> Comments will be welcomed and appreciated!,
> 
> Kind regards,
> guillermo
> 
> --
> 
> BTF supports 64-bits enumerators with following encoding:
> 
>    struct btf_type:
>      name_off: 0 or offset to a valid C identifier
>      info.kind_flag: 0 for unsigned, 1 for signed
>      info.kind: BTF_KIND_ENUM64
>      info.vlen: number of enum values
>      size: 1/2/4/8
> 
> The btf_type is followed by info.vlen number of:
> 
>      struct btf_enum64
>      {
>        uint32_t name_off;   /* Offset in string section of enumerator name.  */
>        uint32_t val_lo32;   /* lower 32-bit value for a 64-bit value Enumerator */
>        uint32_t val_hi32;   /* high 32-bit value for a 64-bit value Enumerator */
>      };
> 
> So, a new btf_enum64 structure was added to represent BTF_KIND_ENUM64
> and a new field dtd_enum_unsigned in ctf_dtdef structure to distinguish
> when CTF enum is a signed or unsigned type, later that information is
> used to encode the BTF enum type.
> 
> gcc/ChangeLog:
> 
> 	* btfout.cc (btf_calc_num_vbytes): Compute enumeration size depending of
> 	enumerator type btf_enum{,64}.
> 	(btf_asm_type): Update btf_kflag according to enumeration type sign
> 	using dtd_enum_unsigned field for both:  BTF_KIND_ENUM{,64}.
> 	(btf_asm_enum_const): New argument to represent the size of
> 	the BTF enum type, writing the enumerator constant value for
> 	32 bits, if it's 64 bits then explicitly writes lower 32-bits
> 	value and higher 32-bits value.
> 	(output_asm_btf_enum_list): Add enumeration size argument.
> 	* ctfc.cc (ctf_add_enum): New argument to represent CTF enum
> 	basic information.
> 	(ctf_add_generic): Use of ei_{name. size, unsigned} to build the
> 	dtd structure containing enumeration information.
> 	(ctf_add_enumerator): Update comment mention support for BTF
> 	enumeration in 64-bits.
> 	* ctfc.h (ctf_dmdef): Update dmd_value to HOST_WIDE_INT to allow
> 	use 32/64 bits enumerators.
> 	(ctf_enum_binfo): New type to represent CTF basic enum type
> 	information.
> 	(ctf_dtdef): New field to describe enum signedness.
> 	* dwarf2ctf.cc (gen_ctf_enumeration_type): Use of ctf_enum_binfo
> 	type to pass information to ctf_add_enum to build the enum type.
> 
> include/
> 	* btf.h (btf_enum64): Add new definition and new symbolic
> 	constant to BTF_KIND_ENUM64 and BTF_KF_ENUM_{UN,}SIGNED.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* gcc.dg/debug/btf/btf-enum-1.c: Update testcase, with correct
> 	info.kflags encoding.
> 	* gcc.dg/debug/btf/btf-enum64-be-1.c: New testcase.
> 	* gcc.dg/debug/btf/btf-enum64-le-1.c: New testcase.
> ---
>   gcc/btfout.cc                                 | 30 ++++++++++---
>   gcc/ctfc.cc                                   | 22 +++++-----
>   gcc/ctfc.h                                    | 15 +++++--
>   gcc/dwarf2ctf.cc                              |  8 +++-
>   gcc/testsuite/gcc.dg/debug/btf/btf-enum-1.c   |  2 +-
>   .../gcc.dg/debug/btf/btf-enum64-be-1.c        | 44 +++++++++++++++++++
>   .../gcc.dg/debug/btf/btf-enum64-le-1.c        | 44 +++++++++++++++++++
>   include/btf.h                                 | 19 ++++++--
>   8 files changed, 160 insertions(+), 24 deletions(-)
>   create mode 100644 gcc/testsuite/gcc.dg/debug/btf/btf-enum64-be-1.c
>   create mode 100644 gcc/testsuite/gcc.dg/debug/btf/btf-enum64-le-1.c
> 
> diff --git a/gcc/btfout.cc b/gcc/btfout.cc
> index 997a33fa089..aef9fd70a28 100644
> --- a/gcc/btfout.cc
> +++ b/gcc/btfout.cc
> @@ -223,7 +223,9 @@ btf_calc_num_vbytes (ctf_dtdef_ref dtd)
>         break;
>   
>       case BTF_KIND_ENUM:
> -      vlen_bytes += vlen * sizeof (struct btf_enum);
> +      vlen_bytes += (dtd->dtd_data.ctti_size == 0x8)
> +			? vlen * sizeof (struct btf_enum64)
> +			: vlen * sizeof (struct btf_enum);
>         break;
>   
>       case BTF_KIND_FUNC_PROTO:
> @@ -622,6 +624,15 @@ btf_asm_type (ctf_container_ref ctfc, ctf_dtdef_ref dtd)
>         btf_size_type = 0;
>       }
>   
> +  if (btf_kind == BTF_KIND_ENUM)
> +    {
> +      btf_kflag = dtd->dtd_enum_unsigned
> +		    ? BTF_KF_ENUM_UNSIGNED
> +		    : BTF_KF_ENUM_SIGNED;
> +      if (dtd->dtd_data.ctti_size == 0x8)
> +	btf_kind = BTF_KIND_ENUM64;
> +   }
> +
>     dw2_asm_output_data (4, dtd->dtd_data.ctti_name, "btt_name");
>     dw2_asm_output_data (4, BTF_TYPE_INFO (btf_kind, btf_kflag, btf_vlen),
>   		       "btt_info: kind=%u, kflag=%u, vlen=%u",
> @@ -634,6 +645,7 @@ btf_asm_type (ctf_container_ref ctfc, ctf_dtdef_ref dtd)
>       case BTF_KIND_UNION:
>       case BTF_KIND_ENUM:
>       case BTF_KIND_DATASEC:
> +    case BTF_KIND_ENUM64:
>         dw2_asm_output_data (4, dtd->dtd_data.ctti_size, "btt_size: %uB",
>   			   dtd->dtd_data.ctti_size);
>         return;
> @@ -707,13 +719,19 @@ btf_asm_sou_member (ctf_container_ref ctfc, ctf_dmdef_t * dmd)
>       }
>   }
>   
> -/* Asm'out an enum constant following a BTF_KIND_ENUM.  */
> +/* Asm'out an enum constant following a BTF_KIND_ENUM{,64}.  */
>   
>   static void
> -btf_asm_enum_const (ctf_dmdef_t * dmd)
> +btf_asm_enum_const (unsigned int size, ctf_dmdef_t * dmd)
>   {
>     dw2_asm_output_data (4, dmd->dmd_name_offset, "bte_name");
> -  dw2_asm_output_data (4, dmd->dmd_value, "bte_value");
> +  if (size == 4)
> +    dw2_asm_output_data (size, dmd->dmd_value, "bte_value");
> +  else
> +    {
> +      dw2_asm_output_data (4, dmd->dmd_value & 0xffffffff, "bte_value_lo32");
> +      dw2_asm_output_data (4, (dmd->dmd_value >> 32) & 0xffffffff, "bte_value_hi32");
> +    }
>   }
>   
>   /* Asm'out a function parameter description following a BTF_KIND_FUNC_PROTO.  */
> @@ -871,7 +889,7 @@ output_asm_btf_sou_fields (ctf_container_ref ctfc, ctf_dtdef_ref dtd)
>         btf_asm_sou_member (ctfc, dmd);
>   }
>   
> -/* Output all enumerator constants following a BTF_KIND_ENUM.  */
> +/* Output all enumerator constants following a BTF_KIND_ENUM{,64}.  */
>   
>   static void
>   output_asm_btf_enum_list (ctf_container_ref ARG_UNUSED (ctfc),
> @@ -881,7 +899,7 @@ output_asm_btf_enum_list (ctf_container_ref ARG_UNUSED (ctfc),
>   
>     for (dmd = dtd->dtd_u.dtu_members;
>          dmd != NULL; dmd = (ctf_dmdef_t *) ctf_dmd_list_next (dmd))
> -    btf_asm_enum_const (dmd);
> +    btf_asm_enum_const (dtd->dtd_data.ctti_size, dmd);
>   }
>   
>   /* Output all function arguments following a BTF_KIND_FUNC_PROTO.  */
> diff --git a/gcc/ctfc.cc b/gcc/ctfc.cc
> index 9773358a475..80322eea91a 100644
> --- a/gcc/ctfc.cc
> +++ b/gcc/ctfc.cc
> @@ -576,8 +576,8 @@ ctf_add_array (ctf_container_ref ctfc, uint32_t flag, const ctf_arinfo_t * arp,
>   }
>   
>   ctf_id_t
> -ctf_add_enum (ctf_container_ref ctfc, uint32_t flag, const char * name,
> -	      HOST_WIDE_INT size, dw_die_ref die)
> +ctf_add_enum (ctf_container_ref ctfc, uint32_t flag,
> +	      const ctf_enum_binfo_t *ei, dw_die_ref die)

The name and size information is typically being passed via explicit 
arguments in the other similar APIs in this functionality.  I have a 
slight preference towards keeping it that way when possible.  So in this 
API ctf_add_enum, how about just adding another function argument for 
signedness and getting rid of the data structure ctf_enum_binfo_t 
altogether. What do you think ?

Patch looks good to me otherwise.

Thanks
Indu

>   {
>     ctf_dtdef_ref dtd;
>     ctf_id_t type;
> @@ -595,16 +595,16 @@ ctf_add_enum (ctf_container_ref ctfc, uint32_t flag, const char * name,
>   	= CTF_TYPE_INFO (CTF_K_FORWARD, CTF_ADD_NONROOT, 0);
>       }
>   
> -  type = ctf_add_generic (ctfc, flag, name, &dtd, die);
> +  type = ctf_add_generic (ctfc, flag, ei->ei_name, &dtd, die);
>   
>     dtd->dtd_data.ctti_info = CTF_TYPE_INFO (CTF_K_ENUM, flag, 0);
>   
>     /* Size in bytes should always fit, of course.
>        TBD WARN - warn instead?  */
> -  gcc_assert (size <= CTF_MAX_SIZE);
> -
> -  dtd->dtd_data.ctti_size = size;
> +  gcc_assert (ei->ei_size <= CTF_MAX_SIZE);
>   
> +  dtd->dtd_data.ctti_size = ei->ei_size;
> +  dtd->dtd_enum_unsigned = ei->ei_unsigned;
>     ctfc->ctfc_num_stypes++;
>   
>     return type;
> @@ -630,10 +630,12 @@ ctf_add_enumerator (ctf_container_ref ctfc, ctf_id_t enid, const char * name,
>   
>     gcc_assert (kind == CTF_K_ENUM && vlen < CTF_MAX_VLEN);
>   
> -  /* Enum value is of type HOST_WIDE_INT in the compiler, dmd_value is int32_t
> -     on the other hand.  Check bounds and skip adding this enum value if out of
> -     bounds.  */
> -  if ((value > INT_MAX) || (value < INT_MIN))
> +  /* Enum value is of type HOST_WIDE_INT in the compiler, CTF enumerators
> +     values in ctf_enum_t is limited to int32_t, BTF supports signed and
> +     unsigned enumerators values of 32 and 64 bits, for both debug formats
> +     we use ctf_dmdef_t.dmd_value entry of HOST_WIDE_INT type. So check
> +     CTF bounds and skip adding this enum value if out of bounds.  */
> +  if (!btf_debuginfo_p() && ((value > INT_MAX) || (value < INT_MIN)))
>       {
>         /* FIXME - Note this TBD_CTF_REPRESENTATION_LIMIT.  */
>         return (1);
> diff --git a/gcc/ctfc.h b/gcc/ctfc.h
> index bcf3a43ae1b..d3f50454385 100644
> --- a/gcc/ctfc.h
> +++ b/gcc/ctfc.h
> @@ -125,6 +125,14 @@ typedef struct GTY (()) ctf_itype
>   
>   #define CTF_FUNC_VARARG 0x1
>   
> +/* Basic enum information to build ctf_dmdef_t type.  */
> +typedef struct ctf_enum_binfo
> +{
> +  const char * ei_name;
> +  unsigned int ei_size;	    /* Size in bytes.  */
> +  bool ei_unsigned;
> +} ctf_enum_binfo_t;
> +
>   /* Struct/union/enum member definition for CTF generation.  */
>   
>   typedef struct GTY ((chain_next ("%h.dmd_next"))) ctf_dmdef
> @@ -133,7 +141,7 @@ typedef struct GTY ((chain_next ("%h.dmd_next"))) ctf_dmdef
>     ctf_id_t dmd_type;		/* Type of this member (for sou).  */
>     uint32_t dmd_name_offset;	/* Offset of the name in str table.  */
>     uint64_t dmd_offset;		/* Offset of this member in bits (for sou).  */
> -  int dmd_value;		/* Value of this member (for enum).  */
> +  HOST_WIDE_INT dmd_value;	/* Value of this member (for enum).  */
>     struct ctf_dmdef * dmd_next;	/* A list node.  */
>   } ctf_dmdef_t;
>   
> @@ -162,6 +170,7 @@ struct GTY ((for_user)) ctf_dtdef
>     bool from_global_func; /* Whether this type was added from a global
>   			    function.  */
>     uint32_t linkage;           /* Used in function types.  0=local, 1=global.  */
> +  bool dtd_enum_unsigned;     /* Enum signedness.  */
>     union GTY ((desc ("ctf_dtu_d_union_selector (&%1)")))
>     {
>       /* struct, union, or enum.  */
> @@ -405,8 +414,8 @@ extern const char * ctf_add_string (ctf_container_ref, const char *,
>   
>   extern ctf_id_t ctf_add_reftype (ctf_container_ref, uint32_t, ctf_id_t,
>   				 uint32_t, dw_die_ref);
> -extern ctf_id_t ctf_add_enum (ctf_container_ref, uint32_t, const char *,
> -			      HOST_WIDE_INT, dw_die_ref);
> +extern ctf_id_t ctf_add_enum (ctf_container_ref, uint32_t,
> +			      const ctf_enum_binfo_t *, dw_die_ref);
>   extern ctf_id_t ctf_add_slice (ctf_container_ref, uint32_t, ctf_id_t,
>   			       uint32_t, uint32_t, dw_die_ref);
>   extern ctf_id_t ctf_add_float (ctf_container_ref, uint32_t, const char *,
> diff --git a/gcc/dwarf2ctf.cc b/gcc/dwarf2ctf.cc
> index 397100004c2..56dfcadd3fb 100644
> --- a/gcc/dwarf2ctf.cc
> +++ b/gcc/dwarf2ctf.cc
> @@ -736,6 +736,7 @@ gen_ctf_enumeration_type (ctf_container_ref ctfc, dw_die_ref enumeration)
>   {
>     const char *enum_name = get_AT_string (enumeration, DW_AT_name);
>     unsigned int bit_size = ctf_die_bitsize (enumeration);
> +  unsigned int signedness = get_AT_unsigned (enumeration, DW_AT_encoding);
>     int declaration_p = get_AT_flag (enumeration, DW_AT_declaration);
>   
>     ctf_id_t enumeration_type_id;
> @@ -757,9 +758,14 @@ gen_ctf_enumeration_type (ctf_container_ref ctfc, dw_die_ref enumeration)
>         bit_size = ctf_die_bitsize (type);
>       }
>   
> +  ctf_enum_binfo_t ei;
> +  ei.ei_name = enum_name;
> +  ei.ei_size = bit_size / 8;
> +  ei.ei_unsigned = (signedness == DW_ATE_unsigned);
> +
>     /* Generate a CTF type for the enumeration.  */
>     enumeration_type_id = ctf_add_enum (ctfc, CTF_ADD_ROOT,
> -				      enum_name, bit_size / 8, enumeration);
> +				      &ei, enumeration);
>   
>     /* Process the enumerators.  */
>     {
> diff --git a/gcc/testsuite/gcc.dg/debug/btf/btf-enum-1.c b/gcc/testsuite/gcc.dg/debug/btf/btf-enum-1.c
> index 728493b0804..7e940529f1b 100644
> --- a/gcc/testsuite/gcc.dg/debug/btf/btf-enum-1.c
> +++ b/gcc/testsuite/gcc.dg/debug/btf/btf-enum-1.c
> @@ -4,7 +4,7 @@
>   /* { dg-options "-O0 -gbtf -dA" } */
>   
>   /* { dg-final { scan-assembler-times "\[\t \]0x6000004\[\t \]+\[^\n\]*btt_info" 1 } } */
> -/* { dg-final { scan-assembler-times "\[\t \]0x6000003\[\t \]+\[^\n\]*btt_info" 1 } } */
> +/* { dg-final { scan-assembler-times "\[\t \]0x86000003\[\t \]+\[^\n\]*btt_info" 1 } } */
>   /* { dg-final { scan-assembler-times "ascii \"QAD.0\"\[\t \]+\[^\n\]*btf_string" 1 } } */
>   /* { dg-final { scan-assembler-times "ascii \"QED.0\"\[\t \]+\[^\n\]*btf_string" 1 } } */
>   /* { dg-final { scan-assembler-times "ascii \"QOD.0\"\[\t \]+\[^\n\]*btf_string" 1 } } */
> diff --git a/gcc/testsuite/gcc.dg/debug/btf/btf-enum64-be-1.c b/gcc/testsuite/gcc.dg/debug/btf/btf-enum64-be-1.c
> new file mode 100644
> index 00000000000..40d44556e16
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/debug/btf/btf-enum64-be-1.c
> @@ -0,0 +1,44 @@
> +/* Test BTF generation for 64 bits enums.  */
> +
> +/* { dg-do compile } */
> +/* { dg-options "-O0 -gbtf -dA -mbig-endian" } */
> +
> +/* { dg-final { scan-assembler-times "\[\t \].size\[\t \]myenum1,\[\t \]8" 1 } } */
> +/* { dg-final { scan-assembler-times "\[\t \].size\[\t \]myenum2,\[\t \]8" 1 } } */
> +/* { dg-final { scan-assembler-times "\[\t \].size\[\t \]myenum3,\[\t \]8" 1 } } */
> +/* { dg-final { scan-assembler-times "\[\t \]0x13000003\[\t \]+\[^\n\]*btt_info" 2 } } */
> +/* { dg-final { scan-assembler-times "\[\t \]0x93000003\[\t \]+\[^\n\]*btt_info" 1 } } */
> +/* { dg-final { scan-assembler-times "\[\t \]0xffffffaa\[\t \]+\[^\n\]*bte_value_lo32" 2 } } */
> +/* { dg-final { scan-assembler-times "\[\t \]0xff\[\t \]+\[^\n\]*bte_value_hi32" 3 } } */
> +/* { dg-final { scan-assembler-times "ascii \"B1.0\"\[\t \]+\[^\n\]*btf_string" 1 } } */
> +/* { dg-final { scan-assembler-times "ascii \"B2.0\"\[\t \]+\[^\n\]*btf_string" 1 } } */
> +/* { dg-final { scan-assembler-times "ascii \"B3.0\"\[\t \]+\[^\n\]*btf_string" 1 } } */
> +/* { dg-final { scan-assembler-times "ascii \"C1.0\"\[\t \]+\[^\n\]*btf_string" 1 } } */
> +/* { dg-final { scan-assembler-times "ascii \"C2.0\"\[\t \]+\[^\n\]*btf_string" 1 } } */
> +/* { dg-final { scan-assembler-times "ascii \"C3.0\"\[\t \]+\[^\n\]*btf_string" 1 } } */
> +/* { dg-final { scan-assembler-times "ascii \"D1.0\"\[\t \]+\[^\n\]*btf_string" 1 } } */
> +/* { dg-final { scan-assembler-times "ascii \"D2.0\"\[\t \]+\[^\n\]*btf_string" 1 } } */
> +/* { dg-final { scan-assembler-times "ascii \"D3.0\"\[\t \]+\[^\n\]*btf_string" 1 } } */
> +/* { dg-final { scan-assembler-times "bte_value_lo32" 9 } } */
> +/* { dg-final { scan-assembler-times "bte_value_hi32" 9 } } */
> +
> +enum default_enum
> +{
> +  B1 = 0xffffffffaa,
> +  B2 = 0xbbbbbbbb,
> +  B3 = 0xaabbccdd,
> +} myenum1 = B1;
> +
> +enum explicit_unsigned
> +{
> +  C1 = 0xffffffffbbUL,
> +  C2 = 0xbbbbbbbb,
> +  C3 = 0xaabbccdd,
> +} myenum2 = C1;
> +
> +enum signed64
> +{
> +  D1 = 0xffffffffaa,
> +  D2 = 0xbbbbbbbb,
> +  D3 = -0x1,
> +} myenum3 = D1;
> diff --git a/gcc/testsuite/gcc.dg/debug/btf/btf-enum64-le-1.c b/gcc/testsuite/gcc.dg/debug/btf/btf-enum64-le-1.c
> new file mode 100644
> index 00000000000..873a50ea174
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/debug/btf/btf-enum64-le-1.c
> @@ -0,0 +1,44 @@
> +/* Test BTF generation for 64 bits enums.  */
> +
> +/* { dg-do compile } */
> +/* { dg-options "-O0 -gbtf -dA -mlittle-endian" } */
> +
> +/* { dg-final { scan-assembler-times "\[\t \].size\[\t \]myenum1,\[\t \]8" 1 } } */
> +/* { dg-final { scan-assembler-times "\[\t \].size\[\t \]myenum2,\[\t \]8" 1 } } */
> +/* { dg-final { scan-assembler-times "\[\t \].size\[\t \]myenum3,\[\t \]8" 1 } } */
> +/* { dg-final { scan-assembler-times "\[\t \]0x13000003\[\t \]+\[^\n\]*btt_info" 2 } } */
> +/* { dg-final { scan-assembler-times "\[\t \]0x93000003\[\t \]+\[^\n\]*btt_info" 1 } } */
> +/* { dg-final { scan-assembler-times "\[\t \]0xffffffaa\[\t \]+\[^\n\]*bte_value_lo32" 2 } } */
> +/* { dg-final { scan-assembler-times "\[\t \]0xff\[\t \]+\[^\n\]*bte_value_hi32" 3 } } */
> +/* { dg-final { scan-assembler-times "ascii \"B1.0\"\[\t \]+\[^\n\]*btf_string" 1 } } */
> +/* { dg-final { scan-assembler-times "ascii \"B2.0\"\[\t \]+\[^\n\]*btf_string" 1 } } */
> +/* { dg-final { scan-assembler-times "ascii \"B3.0\"\[\t \]+\[^\n\]*btf_string" 1 } } */
> +/* { dg-final { scan-assembler-times "ascii \"C1.0\"\[\t \]+\[^\n\]*btf_string" 1 } } */
> +/* { dg-final { scan-assembler-times "ascii \"C2.0\"\[\t \]+\[^\n\]*btf_string" 1 } } */
> +/* { dg-final { scan-assembler-times "ascii \"C3.0\"\[\t \]+\[^\n\]*btf_string" 1 } } */
> +/* { dg-final { scan-assembler-times "ascii \"D1.0\"\[\t \]+\[^\n\]*btf_string" 1 } } */
> +/* { dg-final { scan-assembler-times "ascii \"D2.0\"\[\t \]+\[^\n\]*btf_string" 1 } } */
> +/* { dg-final { scan-assembler-times "ascii \"D3.0\"\[\t \]+\[^\n\]*btf_string" 1 } } */
> +/* { dg-final { scan-assembler-times "bte_value_lo32" 9 } } */
> +/* { dg-final { scan-assembler-times "bte_value_hi32" 9 } } */
> +
> +enum default_enum
> +{
> +  B1 = 0xffffffffaa,
> +  B2 = 0xbbbbbbbb,
> +  B3 = 0xaabbccdd,
> +} myenum1 = B1;
> +
> +enum explicit_unsigned
> +{
> +  C1 = 0xffffffffbbUL,
> +  C2 = 0xbbbbbbbb,
> +  C3 = 0xaabbccdd,
> +} myenum2 = C1;
> +
> +enum signed64
> +{
> +  D1 = 0xffffffffaa,
> +  D2 = 0xbbbbbbbb,
> +  D3 = -0x1,
> +} myenum3 = D1;
> diff --git a/include/btf.h b/include/btf.h
> index 78b551ced23..eba67f9d599 100644
> --- a/include/btf.h
> +++ b/include/btf.h
> @@ -109,7 +109,8 @@ struct btf_type
>   #define BTF_KIND_VAR		14	/* Variable.  */
>   #define BTF_KIND_DATASEC	15	/* Section such as .bss or .data.  */
>   #define BTF_KIND_FLOAT		16	/* Floating point.  */
> -#define BTF_KIND_MAX		BTF_KIND_FLOAT
> +#define BTF_KIND_ENUM64 	19	/* Enumeration up to 64 bits.  */
> +#define BTF_KIND_MAX		BTF_KIND_ENUM64
>   #define NR_BTF_KINDS		(BTF_KIND_MAX + 1)
>   
>   /* For some BTF_KINDs, struct btf_type is immediately followed by
> @@ -130,14 +131,17 @@ struct btf_type
>   #define BTF_INT_BOOL	(1 << 2)
>   
>   /* BTF_KIND_ENUM is followed by VLEN struct btf_enum entries,
> -   which describe the enumerators. Note that BTF currently only
> -   supports signed 32-bit enumerator values.  */
> +   which describe the enumerators. */
>   struct btf_enum
>   {
>     uint32_t name_off;	/* Offset in string section of enumerator name.  */
>     int32_t  val;		/* Enumerator value.  */
>   };
>   
> +/* BTF_KF_ENUM_ holds the flags for kflags in BTF_KIND_ENUM{,64}.  */
> +#define BTF_KF_ENUM_UNSIGNED	(0)
> +#define BTF_KF_ENUM_SIGNED 	(1 << 0)
> +
>   /* BTF_KIND_ARRAY is followed by a single struct btf_array.  */
>   struct btf_array
>   {
> @@ -190,6 +194,15 @@ struct btf_var_secinfo
>     uint32_t size;	/* Size (in bytes) of variable.  */
>   };
>   
> +/* BTF_KIND_ENUM64 is followed by VLEN struct btf_enum64 entries,
> +   which describe the 64 bits enumerators.  */
> +struct btf_enum64
> +{
> +  uint32_t name_off;	/* Offset in string section of enumerator name.  */
> +  uint32_t val_lo32;	/* lower 32-bit value for a 64-bit value Enumerator */
> +  uint32_t val_hi32;	/* high 32-bit value for a 64-bit value Enumerator */
> +};
> +
>   #ifdef	__cplusplus
>   }
>   #endif
> 


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

* Re: [PATCH v3] btf: Add support to BTF_KIND_ENUM64 type
  2022-10-18 21:40     ` Indu Bhagat
@ 2022-10-18 21:48       ` Guillermo E. Martinez
  0 siblings, 0 replies; 15+ messages in thread
From: Guillermo E. Martinez @ 2022-10-18 21:48 UTC (permalink / raw)
  To: Indu Bhagat, gcc-patches; +Cc: david.faust



On 10/18/22 16:40, Indu Bhagat wrote:
> Hi Guillermo,
> 

Hi Indu,

> On 10/14/22 8:55 PM, Guillermo E. Martinez wrote:
>> Hello,
>>
>> The following is patch v3 to update BTF/CTF backend supporting
>> BTF_KIND_ENUM64 type. Changes from v2:
>>
>>    + Add a new `dtd_enum_unsigned' field in `ctf_dtdef' to indicate
>>      signedness of the enum type.
>>    + Fix endianness for representing BTF enum 64-bits enumerators.
>>    + Add {little,big}-endian testcases.
>>
>> Comments will be welcomed and appreciated!,
>>
>> Kind regards,
>> guillermo
>>
>> -- 
>>
>> BTF supports 64-bits enumerators with following encoding:
>>
>>    struct btf_type:
>>      name_off: 0 or offset to a valid C identifier
>>      info.kind_flag: 0 for unsigned, 1 for signed
>>      info.kind: BTF_KIND_ENUM64
>>      info.vlen: number of enum values
>>      size: 1/2/4/8
>>
>> The btf_type is followed by info.vlen number of:
>>
>>      struct btf_enum64
>>      {
>>        uint32_t name_off;   /* Offset in string section of enumerator name.  */
>>        uint32_t val_lo32;   /* lower 32-bit value for a 64-bit value Enumerator */
>>        uint32_t val_hi32;   /* high 32-bit value for a 64-bit value Enumerator */
>>      };
>>
>> So, a new btf_enum64 structure was added to represent BTF_KIND_ENUM64
>> and a new field dtd_enum_unsigned in ctf_dtdef structure to distinguish
>> when CTF enum is a signed or unsigned type, later that information is
>> used to encode the BTF enum type.
>>
>> gcc/ChangeLog:
>>
>>     * btfout.cc (btf_calc_num_vbytes): Compute enumeration size depending of
>>     enumerator type btf_enum{,64}.
>>     (btf_asm_type): Update btf_kflag according to enumeration type sign
>>     using dtd_enum_unsigned field for both:  BTF_KIND_ENUM{,64}.
>>     (btf_asm_enum_const): New argument to represent the size of
>>     the BTF enum type, writing the enumerator constant value for
>>     32 bits, if it's 64 bits then explicitly writes lower 32-bits
>>     value and higher 32-bits value.
>>     (output_asm_btf_enum_list): Add enumeration size argument.
>>     * ctfc.cc (ctf_add_enum): New argument to represent CTF enum
>>     basic information.
>>     (ctf_add_generic): Use of ei_{name. size, unsigned} to build the
>>     dtd structure containing enumeration information.
>>     (ctf_add_enumerator): Update comment mention support for BTF
>>     enumeration in 64-bits.
>>     * ctfc.h (ctf_dmdef): Update dmd_value to HOST_WIDE_INT to allow
>>     use 32/64 bits enumerators.
>>     (ctf_enum_binfo): New type to represent CTF basic enum type
>>     information.
>>     (ctf_dtdef): New field to describe enum signedness.
>>     * dwarf2ctf.cc (gen_ctf_enumeration_type): Use of ctf_enum_binfo
>>     type to pass information to ctf_add_enum to build the enum type.
>>
>> include/
>>     * btf.h (btf_enum64): Add new definition and new symbolic
>>     constant to BTF_KIND_ENUM64 and BTF_KF_ENUM_{UN,}SIGNED.
>>
>> gcc/testsuite/ChangeLog:
>>
>>     * gcc.dg/debug/btf/btf-enum-1.c: Update testcase, with correct
>>     info.kflags encoding.
>>     * gcc.dg/debug/btf/btf-enum64-be-1.c: New testcase.
>>     * gcc.dg/debug/btf/btf-enum64-le-1.c: New testcase.
>> ---
>>   gcc/btfout.cc                                 | 30 ++++++++++---
>>   gcc/ctfc.cc                                   | 22 +++++-----
>>   gcc/ctfc.h                                    | 15 +++++--
>>   gcc/dwarf2ctf.cc                              |  8 +++-
>>   gcc/testsuite/gcc.dg/debug/btf/btf-enum-1.c   |  2 +-
>>   .../gcc.dg/debug/btf/btf-enum64-be-1.c        | 44 +++++++++++++++++++
>>   .../gcc.dg/debug/btf/btf-enum64-le-1.c        | 44 +++++++++++++++++++
>>   include/btf.h                                 | 19 ++++++--
>>   8 files changed, 160 insertions(+), 24 deletions(-)
>>   create mode 100644 gcc/testsuite/gcc.dg/debug/btf/btf-enum64-be-1.c
>>   create mode 100644 gcc/testsuite/gcc.dg/debug/btf/btf-enum64-le-1.c
>>
>> diff --git a/gcc/btfout.cc b/gcc/btfout.cc
>> index 997a33fa089..aef9fd70a28 100644
>> --- a/gcc/btfout.cc
>> +++ b/gcc/btfout.cc
>> @@ -223,7 +223,9 @@ btf_calc_num_vbytes (ctf_dtdef_ref dtd)
>>         break;
>>       case BTF_KIND_ENUM:
>> -      vlen_bytes += vlen * sizeof (struct btf_enum);
>> +      vlen_bytes += (dtd->dtd_data.ctti_size == 0x8)
>> +            ? vlen * sizeof (struct btf_enum64)
>> +            : vlen * sizeof (struct btf_enum);
>>         break;
>>       case BTF_KIND_FUNC_PROTO:
>> @@ -622,6 +624,15 @@ btf_asm_type (ctf_container_ref ctfc, ctf_dtdef_ref dtd)
>>         btf_size_type = 0;
>>       }
>> +  if (btf_kind == BTF_KIND_ENUM)
>> +    {
>> +      btf_kflag = dtd->dtd_enum_unsigned
>> +            ? BTF_KF_ENUM_UNSIGNED
>> +            : BTF_KF_ENUM_SIGNED;
>> +      if (dtd->dtd_data.ctti_size == 0x8)
>> +    btf_kind = BTF_KIND_ENUM64;
>> +   }
>> +
>>     dw2_asm_output_data (4, dtd->dtd_data.ctti_name, "btt_name");
>>     dw2_asm_output_data (4, BTF_TYPE_INFO (btf_kind, btf_kflag, btf_vlen),
>>                  "btt_info: kind=%u, kflag=%u, vlen=%u",
>> @@ -634,6 +645,7 @@ btf_asm_type (ctf_container_ref ctfc, ctf_dtdef_ref dtd)
>>       case BTF_KIND_UNION:
>>       case BTF_KIND_ENUM:
>>       case BTF_KIND_DATASEC:
>> +    case BTF_KIND_ENUM64:
>>         dw2_asm_output_data (4, dtd->dtd_data.ctti_size, "btt_size: %uB",
>>                  dtd->dtd_data.ctti_size);
>>         return;
>> @@ -707,13 +719,19 @@ btf_asm_sou_member (ctf_container_ref ctfc, ctf_dmdef_t * dmd)
>>       }
>>   }
>> -/* Asm'out an enum constant following a BTF_KIND_ENUM.  */
>> +/* Asm'out an enum constant following a BTF_KIND_ENUM{,64}.  */
>>   static void
>> -btf_asm_enum_const (ctf_dmdef_t * dmd)
>> +btf_asm_enum_const (unsigned int size, ctf_dmdef_t * dmd)
>>   {
>>     dw2_asm_output_data (4, dmd->dmd_name_offset, "bte_name");
>> -  dw2_asm_output_data (4, dmd->dmd_value, "bte_value");
>> +  if (size == 4)
>> +    dw2_asm_output_data (size, dmd->dmd_value, "bte_value");
>> +  else
>> +    {
>> +      dw2_asm_output_data (4, dmd->dmd_value & 0xffffffff, "bte_value_lo32");
>> +      dw2_asm_output_data (4, (dmd->dmd_value >> 32) & 0xffffffff, "bte_value_hi32");
>> +    }
>>   }
>>   /* Asm'out a function parameter description following a BTF_KIND_FUNC_PROTO.  */
>> @@ -871,7 +889,7 @@ output_asm_btf_sou_fields (ctf_container_ref ctfc, ctf_dtdef_ref dtd)
>>         btf_asm_sou_member (ctfc, dmd);
>>   }
>> -/* Output all enumerator constants following a BTF_KIND_ENUM.  */
>> +/* Output all enumerator constants following a BTF_KIND_ENUM{,64}.  */
>>   static void
>>   output_asm_btf_enum_list (ctf_container_ref ARG_UNUSED (ctfc),
>> @@ -881,7 +899,7 @@ output_asm_btf_enum_list (ctf_container_ref ARG_UNUSED (ctfc),
>>     for (dmd = dtd->dtd_u.dtu_members;
>>          dmd != NULL; dmd = (ctf_dmdef_t *) ctf_dmd_list_next (dmd))
>> -    btf_asm_enum_const (dmd);
>> +    btf_asm_enum_const (dtd->dtd_data.ctti_size, dmd);
>>   }
>>   /* Output all function arguments following a BTF_KIND_FUNC_PROTO.  */
>> diff --git a/gcc/ctfc.cc b/gcc/ctfc.cc
>> index 9773358a475..80322eea91a 100644
>> --- a/gcc/ctfc.cc
>> +++ b/gcc/ctfc.cc
>> @@ -576,8 +576,8 @@ ctf_add_array (ctf_container_ref ctfc, uint32_t flag, const ctf_arinfo_t * arp,
>>   }
>>   ctf_id_t
>> -ctf_add_enum (ctf_container_ref ctfc, uint32_t flag, const char * name,
>> -          HOST_WIDE_INT size, dw_die_ref die)
>> +ctf_add_enum (ctf_container_ref ctfc, uint32_t flag,
>> +          const ctf_enum_binfo_t *ei, dw_die_ref die)
> 
> The name and size information is typically being passed via explicit arguments in the other similar APIs in this functionality.  I have a slight preference towards keeping it that way when possible.  So in this API ctf_add_enum, how about just adding another function argument for signedness and getting rid of the data structure ctf_enum_binfo_t altogether. What do you think ?
> 

Sure!, I'll send the patch v4 with this change.

> Patch looks good to me otherwise.
>

Thanks for your comments,
  
> Thanks
> Indu
> 
>> [...]
Kind regards,
guillermo

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

* [PATCH v4] btf: Add support to BTF_KIND_ENUM64 type
  2022-10-15  3:55   ` [PATCH v3] " Guillermo E. Martinez
  2022-10-18 21:40     ` Indu Bhagat
@ 2022-10-20  2:05     ` Guillermo E. Martinez
  2022-10-21  9:28       ` Indu Bhagat
  1 sibling, 1 reply; 15+ messages in thread
From: Guillermo E. Martinez @ 2022-10-20  2:05 UTC (permalink / raw)
  To: gcc-patches; +Cc: indu.bhagat, david.faust, Guillermo E. Martinez

Hello,

The following is patch v4 to update BTF/CTF backend supporting
BTF_KIND_ENUM64 type. Changes from v3:

  + Remove `ctf_enum_binfo' structure.
  + Remove -m{little,big}-endian from dg-options in testcase.

Comments will be welcomed and appreciated!,

Kind regards,
guillermo
--

BTF supports 64-bits enumerators with following encoding:

  struct btf_type:
    name_off: 0 or offset to a valid C identifier
    info.kind_flag: 0 for unsigned, 1 for signed
    info.kind: BTF_KIND_ENUM64
    info.vlen: number of enum values
    size: 1/2/4/8

The btf_type is followed by info.vlen number of:

    struct btf_enum64
    {
      uint32_t name_off;   /* Offset in string section of enumerator name.  */
      uint32_t val_lo32;   /* lower 32-bit value for a 64-bit value Enumerator */
      uint32_t val_hi32;   /* high 32-bit value for a 64-bit value Enumerator */
    };

So, a new btf_enum64 structure was added to represent BTF_KIND_ENUM64
and a new field dtd_enum_unsigned in ctf_dtdef structure to distinguish
when CTF enum is a signed or unsigned type, later that information is
used to encode the BTF enum type.

gcc/ChangeLog:

	* btfout.cc (btf_calc_num_vbytes): Compute enumeration size depending of
	enumerator type btf_enum{,64}.
	(btf_asm_type): Update btf_kflag according to enumeration type sign
	using dtd_enum_unsigned field for both:  BTF_KIND_ENUM{,64}.
	(btf_asm_enum_const): New argument to represent the size of
	the BTF enum type, writing the enumerator constant value for
	32 bits, if it's 64 bits then explicitly writes lower 32-bits
	value and higher 32-bits value.
	(output_asm_btf_enum_list): Add enumeration size argument.
	* ctfc.cc (ctf_add_enum): New argument to represent CTF enum
	basic information.
	(ctf_add_generic): Use of ei_{name. size, unsigned} to build the
	dtd structure containing enumeration information.
	(ctf_add_enumerator): Update comment mention support for BTF
	enumeration in 64-bits.
	* dwarf2ctf.cc (gen_ctf_enumeration_type): Extract signedness
	for enumeration type and use it in ctf_add_enum.
	* ctfc.h (ctf_dmdef): Update dmd_value to HOST_WIDE_INT to allow
	use 32/64 bits enumerators.
	information.
	(ctf_dtdef): New field to describe enum signedness.

include/
	* btf.h (btf_enum64): Add new definition and new symbolic
	constant to BTF_KIND_ENUM64 and BTF_KF_ENUM_{UN,}SIGNED.

gcc/testsuite/ChangeLog:

	* gcc.dg/debug/btf/btf-enum-1.c: Update testcase, with correct
	info.kflags encoding.
	* gcc.dg/debug/btf/btf-enum64-1.c: New testcase.
---
 gcc/btfout.cc                                 | 30 ++++++++++---
 gcc/ctfc.cc                                   | 13 +++---
 gcc/ctfc.h                                    |  5 ++-
 gcc/dwarf2ctf.cc                              |  5 ++-
 gcc/testsuite/gcc.dg/debug/btf/btf-enum-1.c   |  2 +-
 gcc/testsuite/gcc.dg/debug/btf/btf-enum64-1.c | 44 +++++++++++++++++++
 include/btf.h                                 | 19 ++++++--
 7 files changed, 100 insertions(+), 18 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/debug/btf/btf-enum64-1.c

diff --git a/gcc/btfout.cc b/gcc/btfout.cc
index 997a33fa089..aef9fd70a28 100644
--- a/gcc/btfout.cc
+++ b/gcc/btfout.cc
@@ -223,7 +223,9 @@ btf_calc_num_vbytes (ctf_dtdef_ref dtd)
       break;
 
     case BTF_KIND_ENUM:
-      vlen_bytes += vlen * sizeof (struct btf_enum);
+      vlen_bytes += (dtd->dtd_data.ctti_size == 0x8)
+			? vlen * sizeof (struct btf_enum64)
+			: vlen * sizeof (struct btf_enum);
       break;
 
     case BTF_KIND_FUNC_PROTO:
@@ -622,6 +624,15 @@ btf_asm_type (ctf_container_ref ctfc, ctf_dtdef_ref dtd)
       btf_size_type = 0;
     }
 
+  if (btf_kind == BTF_KIND_ENUM)
+    {
+      btf_kflag = dtd->dtd_enum_unsigned
+		    ? BTF_KF_ENUM_UNSIGNED
+		    : BTF_KF_ENUM_SIGNED;
+      if (dtd->dtd_data.ctti_size == 0x8)
+	btf_kind = BTF_KIND_ENUM64;
+   }
+
   dw2_asm_output_data (4, dtd->dtd_data.ctti_name, "btt_name");
   dw2_asm_output_data (4, BTF_TYPE_INFO (btf_kind, btf_kflag, btf_vlen),
 		       "btt_info: kind=%u, kflag=%u, vlen=%u",
@@ -634,6 +645,7 @@ btf_asm_type (ctf_container_ref ctfc, ctf_dtdef_ref dtd)
     case BTF_KIND_UNION:
     case BTF_KIND_ENUM:
     case BTF_KIND_DATASEC:
+    case BTF_KIND_ENUM64:
       dw2_asm_output_data (4, dtd->dtd_data.ctti_size, "btt_size: %uB",
 			   dtd->dtd_data.ctti_size);
       return;
@@ -707,13 +719,19 @@ btf_asm_sou_member (ctf_container_ref ctfc, ctf_dmdef_t * dmd)
     }
 }
 
-/* Asm'out an enum constant following a BTF_KIND_ENUM.  */
+/* Asm'out an enum constant following a BTF_KIND_ENUM{,64}.  */
 
 static void
-btf_asm_enum_const (ctf_dmdef_t * dmd)
+btf_asm_enum_const (unsigned int size, ctf_dmdef_t * dmd)
 {
   dw2_asm_output_data (4, dmd->dmd_name_offset, "bte_name");
-  dw2_asm_output_data (4, dmd->dmd_value, "bte_value");
+  if (size == 4)
+    dw2_asm_output_data (size, dmd->dmd_value, "bte_value");
+  else
+    {
+      dw2_asm_output_data (4, dmd->dmd_value & 0xffffffff, "bte_value_lo32");
+      dw2_asm_output_data (4, (dmd->dmd_value >> 32) & 0xffffffff, "bte_value_hi32");
+    }
 }
 
 /* Asm'out a function parameter description following a BTF_KIND_FUNC_PROTO.  */
@@ -871,7 +889,7 @@ output_asm_btf_sou_fields (ctf_container_ref ctfc, ctf_dtdef_ref dtd)
       btf_asm_sou_member (ctfc, dmd);
 }
 
-/* Output all enumerator constants following a BTF_KIND_ENUM.  */
+/* Output all enumerator constants following a BTF_KIND_ENUM{,64}.  */
 
 static void
 output_asm_btf_enum_list (ctf_container_ref ARG_UNUSED (ctfc),
@@ -881,7 +899,7 @@ output_asm_btf_enum_list (ctf_container_ref ARG_UNUSED (ctfc),
 
   for (dmd = dtd->dtd_u.dtu_members;
        dmd != NULL; dmd = (ctf_dmdef_t *) ctf_dmd_list_next (dmd))
-    btf_asm_enum_const (dmd);
+    btf_asm_enum_const (dtd->dtd_data.ctti_size, dmd);
 }
 
 /* Output all function arguments following a BTF_KIND_FUNC_PROTO.  */
diff --git a/gcc/ctfc.cc b/gcc/ctfc.cc
index 9773358a475..34030cbaa49 100644
--- a/gcc/ctfc.cc
+++ b/gcc/ctfc.cc
@@ -577,7 +577,7 @@ ctf_add_array (ctf_container_ref ctfc, uint32_t flag, const ctf_arinfo_t * arp,
 
 ctf_id_t
 ctf_add_enum (ctf_container_ref ctfc, uint32_t flag, const char * name,
-	      HOST_WIDE_INT size, dw_die_ref die)
+	      HOST_WIDE_INT size, bool eunsigned, dw_die_ref die)
 {
   ctf_dtdef_ref dtd;
   ctf_id_t type;
@@ -604,6 +604,7 @@ ctf_add_enum (ctf_container_ref ctfc, uint32_t flag, const char * name,
   gcc_assert (size <= CTF_MAX_SIZE);
 
   dtd->dtd_data.ctti_size = size;
+  dtd->dtd_enum_unsigned = eunsigned;
 
   ctfc->ctfc_num_stypes++;
 
@@ -630,10 +631,12 @@ ctf_add_enumerator (ctf_container_ref ctfc, ctf_id_t enid, const char * name,
 
   gcc_assert (kind == CTF_K_ENUM && vlen < CTF_MAX_VLEN);
 
-  /* Enum value is of type HOST_WIDE_INT in the compiler, dmd_value is int32_t
-     on the other hand.  Check bounds and skip adding this enum value if out of
-     bounds.  */
-  if ((value > INT_MAX) || (value < INT_MIN))
+  /* Enum value is of type HOST_WIDE_INT in the compiler, CTF enumerators
+     values in ctf_enum_t is limited to int32_t, BTF supports signed and
+     unsigned enumerators values of 32 and 64 bits, for both debug formats
+     we use ctf_dmdef_t.dmd_value entry of HOST_WIDE_INT type. So check
+     CTF bounds and skip adding this enum value if out of bounds.  */
+  if (!btf_debuginfo_p() && ((value > INT_MAX) || (value < INT_MIN)))
     {
       /* FIXME - Note this TBD_CTF_REPRESENTATION_LIMIT.  */
       return (1);
diff --git a/gcc/ctfc.h b/gcc/ctfc.h
index bcf3a43ae1b..48c381a008d 100644
--- a/gcc/ctfc.h
+++ b/gcc/ctfc.h
@@ -133,7 +133,7 @@ typedef struct GTY ((chain_next ("%h.dmd_next"))) ctf_dmdef
   ctf_id_t dmd_type;		/* Type of this member (for sou).  */
   uint32_t dmd_name_offset;	/* Offset of the name in str table.  */
   uint64_t dmd_offset;		/* Offset of this member in bits (for sou).  */
-  int dmd_value;		/* Value of this member (for enum).  */
+  HOST_WIDE_INT dmd_value;	/* Value of this member (for enum).  */
   struct ctf_dmdef * dmd_next;	/* A list node.  */
 } ctf_dmdef_t;
 
@@ -162,6 +162,7 @@ struct GTY ((for_user)) ctf_dtdef
   bool from_global_func; /* Whether this type was added from a global
 			    function.  */
   uint32_t linkage;           /* Used in function types.  0=local, 1=global.  */
+  bool dtd_enum_unsigned;     /* Enum signedness.  */
   union GTY ((desc ("ctf_dtu_d_union_selector (&%1)")))
   {
     /* struct, union, or enum.  */
@@ -406,7 +407,7 @@ extern const char * ctf_add_string (ctf_container_ref, const char *,
 extern ctf_id_t ctf_add_reftype (ctf_container_ref, uint32_t, ctf_id_t,
 				 uint32_t, dw_die_ref);
 extern ctf_id_t ctf_add_enum (ctf_container_ref, uint32_t, const char *,
-			      HOST_WIDE_INT, dw_die_ref);
+			      HOST_WIDE_INT, bool, dw_die_ref);
 extern ctf_id_t ctf_add_slice (ctf_container_ref, uint32_t, ctf_id_t,
 			       uint32_t, uint32_t, dw_die_ref);
 extern ctf_id_t ctf_add_float (ctf_container_ref, uint32_t, const char *,
diff --git a/gcc/dwarf2ctf.cc b/gcc/dwarf2ctf.cc
index 397100004c2..748dd0cd8af 100644
--- a/gcc/dwarf2ctf.cc
+++ b/gcc/dwarf2ctf.cc
@@ -736,6 +736,7 @@ gen_ctf_enumeration_type (ctf_container_ref ctfc, dw_die_ref enumeration)
 {
   const char *enum_name = get_AT_string (enumeration, DW_AT_name);
   unsigned int bit_size = ctf_die_bitsize (enumeration);
+  unsigned int signedness = get_AT_unsigned (enumeration, DW_AT_encoding);
   int declaration_p = get_AT_flag (enumeration, DW_AT_declaration);
 
   ctf_id_t enumeration_type_id;
@@ -759,7 +760,9 @@ gen_ctf_enumeration_type (ctf_container_ref ctfc, dw_die_ref enumeration)
 
   /* Generate a CTF type for the enumeration.  */
   enumeration_type_id = ctf_add_enum (ctfc, CTF_ADD_ROOT,
-				      enum_name, bit_size / 8, enumeration);
+				      enum_name, bit_size / 8,
+				      (signedness == DW_ATE_unsigned),
+				      enumeration);
 
   /* Process the enumerators.  */
   {
diff --git a/gcc/testsuite/gcc.dg/debug/btf/btf-enum-1.c b/gcc/testsuite/gcc.dg/debug/btf/btf-enum-1.c
index 728493b0804..7e940529f1b 100644
--- a/gcc/testsuite/gcc.dg/debug/btf/btf-enum-1.c
+++ b/gcc/testsuite/gcc.dg/debug/btf/btf-enum-1.c
@@ -4,7 +4,7 @@
 /* { dg-options "-O0 -gbtf -dA" } */
 
 /* { dg-final { scan-assembler-times "\[\t \]0x6000004\[\t \]+\[^\n\]*btt_info" 1 } } */
-/* { dg-final { scan-assembler-times "\[\t \]0x6000003\[\t \]+\[^\n\]*btt_info" 1 } } */
+/* { dg-final { scan-assembler-times "\[\t \]0x86000003\[\t \]+\[^\n\]*btt_info" 1 } } */
 /* { dg-final { scan-assembler-times "ascii \"QAD.0\"\[\t \]+\[^\n\]*btf_string" 1 } } */
 /* { dg-final { scan-assembler-times "ascii \"QED.0\"\[\t \]+\[^\n\]*btf_string" 1 } } */
 /* { dg-final { scan-assembler-times "ascii \"QOD.0\"\[\t \]+\[^\n\]*btf_string" 1 } } */
diff --git a/gcc/testsuite/gcc.dg/debug/btf/btf-enum64-1.c b/gcc/testsuite/gcc.dg/debug/btf/btf-enum64-1.c
new file mode 100644
index 00000000000..e443d4c8c00
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/debug/btf/btf-enum64-1.c
@@ -0,0 +1,44 @@
+/* Test BTF generation for 64 bits enums.  */
+
+/* { dg-do compile } */
+/* { dg-options "-O0 -gbtf -dA" } */
+
+/* { dg-final { scan-assembler-times "\[\t \].size\[\t \]myenum1,\[\t \]8" 1 } } */
+/* { dg-final { scan-assembler-times "\[\t \].size\[\t \]myenum2,\[\t \]8" 1 } } */
+/* { dg-final { scan-assembler-times "\[\t \].size\[\t \]myenum3,\[\t \]8" 1 } } */
+/* { dg-final { scan-assembler-times "\[\t \]0x13000003\[\t \]+\[^\n\]*btt_info" 2 } } */
+/* { dg-final { scan-assembler-times "\[\t \]0x93000003\[\t \]+\[^\n\]*btt_info" 1 } } */
+/* { dg-final { scan-assembler-times "\[\t \]0xffffffaa\[\t \]+\[^\n\]*bte_value_lo32" 2 } } */
+/* { dg-final { scan-assembler-times "\[\t \]0xff\[\t \]+\[^\n\]*bte_value_hi32" 3 } } */
+/* { dg-final { scan-assembler-times "ascii \"B1.0\"\[\t \]+\[^\n\]*btf_string" 1 } } */
+/* { dg-final { scan-assembler-times "ascii \"B2.0\"\[\t \]+\[^\n\]*btf_string" 1 } } */
+/* { dg-final { scan-assembler-times "ascii \"B3.0\"\[\t \]+\[^\n\]*btf_string" 1 } } */
+/* { dg-final { scan-assembler-times "ascii \"C1.0\"\[\t \]+\[^\n\]*btf_string" 1 } } */
+/* { dg-final { scan-assembler-times "ascii \"C2.0\"\[\t \]+\[^\n\]*btf_string" 1 } } */
+/* { dg-final { scan-assembler-times "ascii \"C3.0\"\[\t \]+\[^\n\]*btf_string" 1 } } */
+/* { dg-final { scan-assembler-times "ascii \"D1.0\"\[\t \]+\[^\n\]*btf_string" 1 } } */
+/* { dg-final { scan-assembler-times "ascii \"D2.0\"\[\t \]+\[^\n\]*btf_string" 1 } } */
+/* { dg-final { scan-assembler-times "ascii \"D3.0\"\[\t \]+\[^\n\]*btf_string" 1 } } */
+/* { dg-final { scan-assembler-times "bte_value_lo32" 9 } } */
+/* { dg-final { scan-assembler-times "bte_value_hi32" 9 } } */
+
+enum default_enum
+{
+  B1 = 0xffffffffaa,
+  B2 = 0xbbbbbbbb,
+  B3 = 0xaabbccdd,
+} myenum1 = B1;
+
+enum explicit_unsigned
+{
+  C1 = 0xffffffffbbUL,
+  C2 = 0xbbbbbbbb,
+  C3 = 0xaabbccdd,
+} myenum2 = C1;
+
+enum signed64
+{
+  D1 = 0xffffffffaa,
+  D2 = 0xbbbbbbbb,
+  D3 = -0x1,
+} myenum3 = D1;
diff --git a/include/btf.h b/include/btf.h
index 78b551ced23..eba67f9d599 100644
--- a/include/btf.h
+++ b/include/btf.h
@@ -109,7 +109,8 @@ struct btf_type
 #define BTF_KIND_VAR		14	/* Variable.  */
 #define BTF_KIND_DATASEC	15	/* Section such as .bss or .data.  */
 #define BTF_KIND_FLOAT		16	/* Floating point.  */
-#define BTF_KIND_MAX		BTF_KIND_FLOAT
+#define BTF_KIND_ENUM64 	19	/* Enumeration up to 64 bits.  */
+#define BTF_KIND_MAX		BTF_KIND_ENUM64
 #define NR_BTF_KINDS		(BTF_KIND_MAX + 1)
 
 /* For some BTF_KINDs, struct btf_type is immediately followed by
@@ -130,14 +131,17 @@ struct btf_type
 #define BTF_INT_BOOL	(1 << 2)
 
 /* BTF_KIND_ENUM is followed by VLEN struct btf_enum entries,
-   which describe the enumerators. Note that BTF currently only
-   supports signed 32-bit enumerator values.  */
+   which describe the enumerators. */
 struct btf_enum
 {
   uint32_t name_off;	/* Offset in string section of enumerator name.  */
   int32_t  val;		/* Enumerator value.  */
 };
 
+/* BTF_KF_ENUM_ holds the flags for kflags in BTF_KIND_ENUM{,64}.  */
+#define BTF_KF_ENUM_UNSIGNED	(0)
+#define BTF_KF_ENUM_SIGNED 	(1 << 0)
+
 /* BTF_KIND_ARRAY is followed by a single struct btf_array.  */
 struct btf_array
 {
@@ -190,6 +194,15 @@ struct btf_var_secinfo
   uint32_t size;	/* Size (in bytes) of variable.  */
 };
 
+/* BTF_KIND_ENUM64 is followed by VLEN struct btf_enum64 entries,
+   which describe the 64 bits enumerators.  */
+struct btf_enum64
+{
+  uint32_t name_off;	/* Offset in string section of enumerator name.  */
+  uint32_t val_lo32;	/* lower 32-bit value for a 64-bit value Enumerator */
+  uint32_t val_hi32;	/* high 32-bit value for a 64-bit value Enumerator */
+};
+
 #ifdef	__cplusplus
 }
 #endif
-- 
2.35.1


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

* Re: [PATCH v4] btf: Add support to BTF_KIND_ENUM64 type
  2022-10-20  2:05     ` [PATCH v4] " Guillermo E. Martinez
@ 2022-10-21  9:28       ` Indu Bhagat
  2022-10-31 18:26         ` Indu Bhagat
  0 siblings, 1 reply; 15+ messages in thread
From: Indu Bhagat @ 2022-10-21  9:28 UTC (permalink / raw)
  To: Guillermo E. Martinez, gcc-patches; +Cc: david.faust

On 10/19/22 19:05, Guillermo E. Martinez wrote:
> Hello,
> 
> The following is patch v4 to update BTF/CTF backend supporting
> BTF_KIND_ENUM64 type. Changes from v3:
> 
>    + Remove `ctf_enum_binfo' structure.
>    + Remove -m{little,big}-endian from dg-options in testcase.
> 
> Comments will be welcomed and appreciated!,
> 
> Kind regards,
> guillermo
> --
> 

Thanks Guillermo.

LGTM.

> BTF supports 64-bits enumerators with following encoding:
> 
>    struct btf_type:
>      name_off: 0 or offset to a valid C identifier
>      info.kind_flag: 0 for unsigned, 1 for signed
>      info.kind: BTF_KIND_ENUM64
>      info.vlen: number of enum values
>      size: 1/2/4/8
> 
> The btf_type is followed by info.vlen number of:
> 
>      struct btf_enum64
>      {
>        uint32_t name_off;   /* Offset in string section of enumerator name.  */
>        uint32_t val_lo32;   /* lower 32-bit value for a 64-bit value Enumerator */
>        uint32_t val_hi32;   /* high 32-bit value for a 64-bit value Enumerator */
>      };
> 
> So, a new btf_enum64 structure was added to represent BTF_KIND_ENUM64
> and a new field dtd_enum_unsigned in ctf_dtdef structure to distinguish
> when CTF enum is a signed or unsigned type, later that information is
> used to encode the BTF enum type.
> 
> gcc/ChangeLog:
> 
> 	* btfout.cc (btf_calc_num_vbytes): Compute enumeration size depending of
> 	enumerator type btf_enum{,64}.
> 	(btf_asm_type): Update btf_kflag according to enumeration type sign
> 	using dtd_enum_unsigned field for both:  BTF_KIND_ENUM{,64}.
> 	(btf_asm_enum_const): New argument to represent the size of
> 	the BTF enum type, writing the enumerator constant value for
> 	32 bits, if it's 64 bits then explicitly writes lower 32-bits
> 	value and higher 32-bits value.
> 	(output_asm_btf_enum_list): Add enumeration size argument.
> 	* ctfc.cc (ctf_add_enum): New argument to represent CTF enum
> 	basic information.
> 	(ctf_add_generic): Use of ei_{name. size, unsigned} to build the
> 	dtd structure containing enumeration information.
> 	(ctf_add_enumerator): Update comment mention support for BTF
> 	enumeration in 64-bits.
> 	* dwarf2ctf.cc (gen_ctf_enumeration_type): Extract signedness
> 	for enumeration type and use it in ctf_add_enum.
> 	* ctfc.h (ctf_dmdef): Update dmd_value to HOST_WIDE_INT to allow
> 	use 32/64 bits enumerators.
> 	information.
> 	(ctf_dtdef): New field to describe enum signedness.
> 
> include/
> 	* btf.h (btf_enum64): Add new definition and new symbolic
> 	constant to BTF_KIND_ENUM64 and BTF_KF_ENUM_{UN,}SIGNED.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* gcc.dg/debug/btf/btf-enum-1.c: Update testcase, with correct
> 	info.kflags encoding.
> 	* gcc.dg/debug/btf/btf-enum64-1.c: New testcase.
> ---
>   gcc/btfout.cc                                 | 30 ++++++++++---
>   gcc/ctfc.cc                                   | 13 +++---
>   gcc/ctfc.h                                    |  5 ++-
>   gcc/dwarf2ctf.cc                              |  5 ++-
>   gcc/testsuite/gcc.dg/debug/btf/btf-enum-1.c   |  2 +-
>   gcc/testsuite/gcc.dg/debug/btf/btf-enum64-1.c | 44 +++++++++++++++++++
>   include/btf.h                                 | 19 ++++++--
>   7 files changed, 100 insertions(+), 18 deletions(-)
>   create mode 100644 gcc/testsuite/gcc.dg/debug/btf/btf-enum64-1.c
> 
> diff --git a/gcc/btfout.cc b/gcc/btfout.cc
> index 997a33fa089..aef9fd70a28 100644
> --- a/gcc/btfout.cc
> +++ b/gcc/btfout.cc
> @@ -223,7 +223,9 @@ btf_calc_num_vbytes (ctf_dtdef_ref dtd)
>         break;
>   
>       case BTF_KIND_ENUM:
> -      vlen_bytes += vlen * sizeof (struct btf_enum);
> +      vlen_bytes += (dtd->dtd_data.ctti_size == 0x8)
> +			? vlen * sizeof (struct btf_enum64)
> +			: vlen * sizeof (struct btf_enum);
>         break;
>   
>       case BTF_KIND_FUNC_PROTO:
> @@ -622,6 +624,15 @@ btf_asm_type (ctf_container_ref ctfc, ctf_dtdef_ref dtd)
>         btf_size_type = 0;
>       }
>   
> +  if (btf_kind == BTF_KIND_ENUM)
> +    {
> +      btf_kflag = dtd->dtd_enum_unsigned
> +		    ? BTF_KF_ENUM_UNSIGNED
> +		    : BTF_KF_ENUM_SIGNED;
> +      if (dtd->dtd_data.ctti_size == 0x8)
> +	btf_kind = BTF_KIND_ENUM64;
> +   }
> +
>     dw2_asm_output_data (4, dtd->dtd_data.ctti_name, "btt_name");
>     dw2_asm_output_data (4, BTF_TYPE_INFO (btf_kind, btf_kflag, btf_vlen),
>   		       "btt_info: kind=%u, kflag=%u, vlen=%u",
> @@ -634,6 +645,7 @@ btf_asm_type (ctf_container_ref ctfc, ctf_dtdef_ref dtd)
>       case BTF_KIND_UNION:
>       case BTF_KIND_ENUM:
>       case BTF_KIND_DATASEC:
> +    case BTF_KIND_ENUM64:
>         dw2_asm_output_data (4, dtd->dtd_data.ctti_size, "btt_size: %uB",
>   			   dtd->dtd_data.ctti_size);
>         return;
> @@ -707,13 +719,19 @@ btf_asm_sou_member (ctf_container_ref ctfc, ctf_dmdef_t * dmd)
>       }
>   }
>   
> -/* Asm'out an enum constant following a BTF_KIND_ENUM.  */
> +/* Asm'out an enum constant following a BTF_KIND_ENUM{,64}.  */
>   
>   static void
> -btf_asm_enum_const (ctf_dmdef_t * dmd)
> +btf_asm_enum_const (unsigned int size, ctf_dmdef_t * dmd)
>   {
>     dw2_asm_output_data (4, dmd->dmd_name_offset, "bte_name");
> -  dw2_asm_output_data (4, dmd->dmd_value, "bte_value");
> +  if (size == 4)
> +    dw2_asm_output_data (size, dmd->dmd_value, "bte_value");
> +  else
> +    {
> +      dw2_asm_output_data (4, dmd->dmd_value & 0xffffffff, "bte_value_lo32");
> +      dw2_asm_output_data (4, (dmd->dmd_value >> 32) & 0xffffffff, "bte_value_hi32");
> +    }
>   }
>   
>   /* Asm'out a function parameter description following a BTF_KIND_FUNC_PROTO.  */
> @@ -871,7 +889,7 @@ output_asm_btf_sou_fields (ctf_container_ref ctfc, ctf_dtdef_ref dtd)
>         btf_asm_sou_member (ctfc, dmd);
>   }
>   
> -/* Output all enumerator constants following a BTF_KIND_ENUM.  */
> +/* Output all enumerator constants following a BTF_KIND_ENUM{,64}.  */
>   
>   static void
>   output_asm_btf_enum_list (ctf_container_ref ARG_UNUSED (ctfc),
> @@ -881,7 +899,7 @@ output_asm_btf_enum_list (ctf_container_ref ARG_UNUSED (ctfc),
>   
>     for (dmd = dtd->dtd_u.dtu_members;
>          dmd != NULL; dmd = (ctf_dmdef_t *) ctf_dmd_list_next (dmd))
> -    btf_asm_enum_const (dmd);
> +    btf_asm_enum_const (dtd->dtd_data.ctti_size, dmd);
>   }
>   
>   /* Output all function arguments following a BTF_KIND_FUNC_PROTO.  */
> diff --git a/gcc/ctfc.cc b/gcc/ctfc.cc
> index 9773358a475..34030cbaa49 100644
> --- a/gcc/ctfc.cc
> +++ b/gcc/ctfc.cc
> @@ -577,7 +577,7 @@ ctf_add_array (ctf_container_ref ctfc, uint32_t flag, const ctf_arinfo_t * arp,
>   
>   ctf_id_t
>   ctf_add_enum (ctf_container_ref ctfc, uint32_t flag, const char * name,
> -	      HOST_WIDE_INT size, dw_die_ref die)
> +	      HOST_WIDE_INT size, bool eunsigned, dw_die_ref die)
>   {
>     ctf_dtdef_ref dtd;
>     ctf_id_t type;
> @@ -604,6 +604,7 @@ ctf_add_enum (ctf_container_ref ctfc, uint32_t flag, const char * name,
>     gcc_assert (size <= CTF_MAX_SIZE);
>   
>     dtd->dtd_data.ctti_size = size;
> +  dtd->dtd_enum_unsigned = eunsigned;
>   
>     ctfc->ctfc_num_stypes++;
>   
> @@ -630,10 +631,12 @@ ctf_add_enumerator (ctf_container_ref ctfc, ctf_id_t enid, const char * name,
>   
>     gcc_assert (kind == CTF_K_ENUM && vlen < CTF_MAX_VLEN);
>   
> -  /* Enum value is of type HOST_WIDE_INT in the compiler, dmd_value is int32_t
> -     on the other hand.  Check bounds and skip adding this enum value if out of
> -     bounds.  */
> -  if ((value > INT_MAX) || (value < INT_MIN))
> +  /* Enum value is of type HOST_WIDE_INT in the compiler, CTF enumerators
> +     values in ctf_enum_t is limited to int32_t, BTF supports signed and
> +     unsigned enumerators values of 32 and 64 bits, for both debug formats
> +     we use ctf_dmdef_t.dmd_value entry of HOST_WIDE_INT type. So check
> +     CTF bounds and skip adding this enum value if out of bounds.  */
> +  if (!btf_debuginfo_p() && ((value > INT_MAX) || (value < INT_MIN)))
>       {
>         /* FIXME - Note this TBD_CTF_REPRESENTATION_LIMIT.  */
>         return (1);
> diff --git a/gcc/ctfc.h b/gcc/ctfc.h
> index bcf3a43ae1b..48c381a008d 100644
> --- a/gcc/ctfc.h
> +++ b/gcc/ctfc.h
> @@ -133,7 +133,7 @@ typedef struct GTY ((chain_next ("%h.dmd_next"))) ctf_dmdef
>     ctf_id_t dmd_type;		/* Type of this member (for sou).  */
>     uint32_t dmd_name_offset;	/* Offset of the name in str table.  */
>     uint64_t dmd_offset;		/* Offset of this member in bits (for sou).  */
> -  int dmd_value;		/* Value of this member (for enum).  */
> +  HOST_WIDE_INT dmd_value;	/* Value of this member (for enum).  */
>     struct ctf_dmdef * dmd_next;	/* A list node.  */
>   } ctf_dmdef_t;
>   
> @@ -162,6 +162,7 @@ struct GTY ((for_user)) ctf_dtdef
>     bool from_global_func; /* Whether this type was added from a global
>   			    function.  */
>     uint32_t linkage;           /* Used in function types.  0=local, 1=global.  */
> +  bool dtd_enum_unsigned;     /* Enum signedness.  */
>     union GTY ((desc ("ctf_dtu_d_union_selector (&%1)")))
>     {
>       /* struct, union, or enum.  */
> @@ -406,7 +407,7 @@ extern const char * ctf_add_string (ctf_container_ref, const char *,
>   extern ctf_id_t ctf_add_reftype (ctf_container_ref, uint32_t, ctf_id_t,
>   				 uint32_t, dw_die_ref);
>   extern ctf_id_t ctf_add_enum (ctf_container_ref, uint32_t, const char *,
> -			      HOST_WIDE_INT, dw_die_ref);
> +			      HOST_WIDE_INT, bool, dw_die_ref);
>   extern ctf_id_t ctf_add_slice (ctf_container_ref, uint32_t, ctf_id_t,
>   			       uint32_t, uint32_t, dw_die_ref);
>   extern ctf_id_t ctf_add_float (ctf_container_ref, uint32_t, const char *,
> diff --git a/gcc/dwarf2ctf.cc b/gcc/dwarf2ctf.cc
> index 397100004c2..748dd0cd8af 100644
> --- a/gcc/dwarf2ctf.cc
> +++ b/gcc/dwarf2ctf.cc
> @@ -736,6 +736,7 @@ gen_ctf_enumeration_type (ctf_container_ref ctfc, dw_die_ref enumeration)
>   {
>     const char *enum_name = get_AT_string (enumeration, DW_AT_name);
>     unsigned int bit_size = ctf_die_bitsize (enumeration);
> +  unsigned int signedness = get_AT_unsigned (enumeration, DW_AT_encoding);
>     int declaration_p = get_AT_flag (enumeration, DW_AT_declaration);
>   
>     ctf_id_t enumeration_type_id;
> @@ -759,7 +760,9 @@ gen_ctf_enumeration_type (ctf_container_ref ctfc, dw_die_ref enumeration)
>   
>     /* Generate a CTF type for the enumeration.  */
>     enumeration_type_id = ctf_add_enum (ctfc, CTF_ADD_ROOT,
> -				      enum_name, bit_size / 8, enumeration);
> +				      enum_name, bit_size / 8,
> +				      (signedness == DW_ATE_unsigned),
> +				      enumeration);
>   
>     /* Process the enumerators.  */
>     {
> diff --git a/gcc/testsuite/gcc.dg/debug/btf/btf-enum-1.c b/gcc/testsuite/gcc.dg/debug/btf/btf-enum-1.c
> index 728493b0804..7e940529f1b 100644
> --- a/gcc/testsuite/gcc.dg/debug/btf/btf-enum-1.c
> +++ b/gcc/testsuite/gcc.dg/debug/btf/btf-enum-1.c
> @@ -4,7 +4,7 @@
>   /* { dg-options "-O0 -gbtf -dA" } */
>   
>   /* { dg-final { scan-assembler-times "\[\t \]0x6000004\[\t \]+\[^\n\]*btt_info" 1 } } */
> -/* { dg-final { scan-assembler-times "\[\t \]0x6000003\[\t \]+\[^\n\]*btt_info" 1 } } */
> +/* { dg-final { scan-assembler-times "\[\t \]0x86000003\[\t \]+\[^\n\]*btt_info" 1 } } */
>   /* { dg-final { scan-assembler-times "ascii \"QAD.0\"\[\t \]+\[^\n\]*btf_string" 1 } } */
>   /* { dg-final { scan-assembler-times "ascii \"QED.0\"\[\t \]+\[^\n\]*btf_string" 1 } } */
>   /* { dg-final { scan-assembler-times "ascii \"QOD.0\"\[\t \]+\[^\n\]*btf_string" 1 } } */
> diff --git a/gcc/testsuite/gcc.dg/debug/btf/btf-enum64-1.c b/gcc/testsuite/gcc.dg/debug/btf/btf-enum64-1.c
> new file mode 100644
> index 00000000000..e443d4c8c00
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/debug/btf/btf-enum64-1.c
> @@ -0,0 +1,44 @@
> +/* Test BTF generation for 64 bits enums.  */
> +
> +/* { dg-do compile } */
> +/* { dg-options "-O0 -gbtf -dA" } */
> +
> +/* { dg-final { scan-assembler-times "\[\t \].size\[\t \]myenum1,\[\t \]8" 1 } } */
> +/* { dg-final { scan-assembler-times "\[\t \].size\[\t \]myenum2,\[\t \]8" 1 } } */
> +/* { dg-final { scan-assembler-times "\[\t \].size\[\t \]myenum3,\[\t \]8" 1 } } */
> +/* { dg-final { scan-assembler-times "\[\t \]0x13000003\[\t \]+\[^\n\]*btt_info" 2 } } */
> +/* { dg-final { scan-assembler-times "\[\t \]0x93000003\[\t \]+\[^\n\]*btt_info" 1 } } */
> +/* { dg-final { scan-assembler-times "\[\t \]0xffffffaa\[\t \]+\[^\n\]*bte_value_lo32" 2 } } */
> +/* { dg-final { scan-assembler-times "\[\t \]0xff\[\t \]+\[^\n\]*bte_value_hi32" 3 } } */
> +/* { dg-final { scan-assembler-times "ascii \"B1.0\"\[\t \]+\[^\n\]*btf_string" 1 } } */
> +/* { dg-final { scan-assembler-times "ascii \"B2.0\"\[\t \]+\[^\n\]*btf_string" 1 } } */
> +/* { dg-final { scan-assembler-times "ascii \"B3.0\"\[\t \]+\[^\n\]*btf_string" 1 } } */
> +/* { dg-final { scan-assembler-times "ascii \"C1.0\"\[\t \]+\[^\n\]*btf_string" 1 } } */
> +/* { dg-final { scan-assembler-times "ascii \"C2.0\"\[\t \]+\[^\n\]*btf_string" 1 } } */
> +/* { dg-final { scan-assembler-times "ascii \"C3.0\"\[\t \]+\[^\n\]*btf_string" 1 } } */
> +/* { dg-final { scan-assembler-times "ascii \"D1.0\"\[\t \]+\[^\n\]*btf_string" 1 } } */
> +/* { dg-final { scan-assembler-times "ascii \"D2.0\"\[\t \]+\[^\n\]*btf_string" 1 } } */
> +/* { dg-final { scan-assembler-times "ascii \"D3.0\"\[\t \]+\[^\n\]*btf_string" 1 } } */
> +/* { dg-final { scan-assembler-times "bte_value_lo32" 9 } } */
> +/* { dg-final { scan-assembler-times "bte_value_hi32" 9 } } */
> +
> +enum default_enum
> +{
> +  B1 = 0xffffffffaa,
> +  B2 = 0xbbbbbbbb,
> +  B3 = 0xaabbccdd,
> +} myenum1 = B1;
> +
> +enum explicit_unsigned
> +{
> +  C1 = 0xffffffffbbUL,
> +  C2 = 0xbbbbbbbb,
> +  C3 = 0xaabbccdd,
> +} myenum2 = C1;
> +
> +enum signed64
> +{
> +  D1 = 0xffffffffaa,
> +  D2 = 0xbbbbbbbb,
> +  D3 = -0x1,
> +} myenum3 = D1;
> diff --git a/include/btf.h b/include/btf.h
> index 78b551ced23..eba67f9d599 100644
> --- a/include/btf.h
> +++ b/include/btf.h
> @@ -109,7 +109,8 @@ struct btf_type
>   #define BTF_KIND_VAR		14	/* Variable.  */
>   #define BTF_KIND_DATASEC	15	/* Section such as .bss or .data.  */
>   #define BTF_KIND_FLOAT		16	/* Floating point.  */
> -#define BTF_KIND_MAX		BTF_KIND_FLOAT
> +#define BTF_KIND_ENUM64 	19	/* Enumeration up to 64 bits.  */
> +#define BTF_KIND_MAX		BTF_KIND_ENUM64
>   #define NR_BTF_KINDS		(BTF_KIND_MAX + 1)
>   
>   /* For some BTF_KINDs, struct btf_type is immediately followed by
> @@ -130,14 +131,17 @@ struct btf_type
>   #define BTF_INT_BOOL	(1 << 2)
>   
>   /* BTF_KIND_ENUM is followed by VLEN struct btf_enum entries,
> -   which describe the enumerators. Note that BTF currently only
> -   supports signed 32-bit enumerator values.  */
> +   which describe the enumerators. */
>   struct btf_enum
>   {
>     uint32_t name_off;	/* Offset in string section of enumerator name.  */
>     int32_t  val;		/* Enumerator value.  */
>   };
>   
> +/* BTF_KF_ENUM_ holds the flags for kflags in BTF_KIND_ENUM{,64}.  */
> +#define BTF_KF_ENUM_UNSIGNED	(0)
> +#define BTF_KF_ENUM_SIGNED 	(1 << 0)
> +
>   /* BTF_KIND_ARRAY is followed by a single struct btf_array.  */
>   struct btf_array
>   {
> @@ -190,6 +194,15 @@ struct btf_var_secinfo
>     uint32_t size;	/* Size (in bytes) of variable.  */
>   };
>   
> +/* BTF_KIND_ENUM64 is followed by VLEN struct btf_enum64 entries,
> +   which describe the 64 bits enumerators.  */
> +struct btf_enum64
> +{
> +  uint32_t name_off;	/* Offset in string section of enumerator name.  */
> +  uint32_t val_lo32;	/* lower 32-bit value for a 64-bit value Enumerator */
> +  uint32_t val_hi32;	/* high 32-bit value for a 64-bit value Enumerator */
> +};
> +
>   #ifdef	__cplusplus
>   }
>   #endif


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

* Re: [PATCH v4] btf: Add support to BTF_KIND_ENUM64 type
  2022-10-21  9:28       ` Indu Bhagat
@ 2022-10-31 18:26         ` Indu Bhagat
  0 siblings, 0 replies; 15+ messages in thread
From: Indu Bhagat @ 2022-10-31 18:26 UTC (permalink / raw)
  To: Indu Bhagat, Guillermo E. Martinez, gcc-patches; +Cc: Jose E. Marchesi

On 10/21/22 2:28 AM, Indu Bhagat via Gcc-patches wrote:
> On 10/19/22 19:05, Guillermo E. Martinez wrote:
>> Hello,
>>
>> The following is patch v4 to update BTF/CTF backend supporting
>> BTF_KIND_ENUM64 type. Changes from v3:
>>
>>    + Remove `ctf_enum_binfo' structure.
>>    + Remove -m{little,big}-endian from dg-options in testcase.
>>
>> Comments will be welcomed and appreciated!,
>>
>> Kind regards,
>> guillermo
>> -- 
>>
> 
> Thanks Guillermo.
> 
> LGTM.
> 

Pushed on behalf of Guillermo.

Thanks

>> BTF supports 64-bits enumerators with following encoding:
>>
>>    struct btf_type:
>>      name_off: 0 or offset to a valid C identifier
>>      info.kind_flag: 0 for unsigned, 1 for signed
>>      info.kind: BTF_KIND_ENUM64
>>      info.vlen: number of enum values
>>      size: 1/2/4/8
>>
>> The btf_type is followed by info.vlen number of:
>>
>>      struct btf_enum64
>>      {
>>        uint32_t name_off;   /* Offset in string section of enumerator 
>> name.  */
>>        uint32_t val_lo32;   /* lower 32-bit value for a 64-bit value 
>> Enumerator */
>>        uint32_t val_hi32;   /* high 32-bit value for a 64-bit value 
>> Enumerator */
>>      };
>>
>> So, a new btf_enum64 structure was added to represent BTF_KIND_ENUM64
>> and a new field dtd_enum_unsigned in ctf_dtdef structure to distinguish
>> when CTF enum is a signed or unsigned type, later that information is
>> used to encode the BTF enum type.
>>
>> gcc/ChangeLog:
>>
>>     * btfout.cc (btf_calc_num_vbytes): Compute enumeration size 
>> depending of
>>     enumerator type btf_enum{,64}.
>>     (btf_asm_type): Update btf_kflag according to enumeration type sign
>>     using dtd_enum_unsigned field for both:  BTF_KIND_ENUM{,64}.
>>     (btf_asm_enum_const): New argument to represent the size of
>>     the BTF enum type, writing the enumerator constant value for
>>     32 bits, if it's 64 bits then explicitly writes lower 32-bits
>>     value and higher 32-bits value.
>>     (output_asm_btf_enum_list): Add enumeration size argument.
>>     * ctfc.cc (ctf_add_enum): New argument to represent CTF enum
>>     basic information.
>>     (ctf_add_generic): Use of ei_{name. size, unsigned} to build the
>>     dtd structure containing enumeration information.
>>     (ctf_add_enumerator): Update comment mention support for BTF
>>     enumeration in 64-bits.
>>     * dwarf2ctf.cc (gen_ctf_enumeration_type): Extract signedness
>>     for enumeration type and use it in ctf_add_enum.
>>     * ctfc.h (ctf_dmdef): Update dmd_value to HOST_WIDE_INT to allow
>>     use 32/64 bits enumerators.
>>     information.
>>     (ctf_dtdef): New field to describe enum signedness.
>>
>> include/
>>     * btf.h (btf_enum64): Add new definition and new symbolic
>>     constant to BTF_KIND_ENUM64 and BTF_KF_ENUM_{UN,}SIGNED.
>>
>> gcc/testsuite/ChangeLog:
>>
>>     * gcc.dg/debug/btf/btf-enum-1.c: Update testcase, with correct
>>     info.kflags encoding.
>>     * gcc.dg/debug/btf/btf-enum64-1.c: New testcase.
>> ---
>>   gcc/btfout.cc                                 | 30 ++++++++++---
>>   gcc/ctfc.cc                                   | 13 +++---
>>   gcc/ctfc.h                                    |  5 ++-
>>   gcc/dwarf2ctf.cc                              |  5 ++-
>>   gcc/testsuite/gcc.dg/debug/btf/btf-enum-1.c   |  2 +-
>>   gcc/testsuite/gcc.dg/debug/btf/btf-enum64-1.c | 44 +++++++++++++++++++
>>   include/btf.h                                 | 19 ++++++--
>>   7 files changed, 100 insertions(+), 18 deletions(-)
>>   create mode 100644 gcc/testsuite/gcc.dg/debug/btf/btf-enum64-1.c
>>
>> diff --git a/gcc/btfout.cc b/gcc/btfout.cc
>> index 997a33fa089..aef9fd70a28 100644
>> --- a/gcc/btfout.cc
>> +++ b/gcc/btfout.cc
>> @@ -223,7 +223,9 @@ btf_calc_num_vbytes (ctf_dtdef_ref dtd)
>>         break;
>>       case BTF_KIND_ENUM:
>> -      vlen_bytes += vlen * sizeof (struct btf_enum);
>> +      vlen_bytes += (dtd->dtd_data.ctti_size == 0x8)
>> +            ? vlen * sizeof (struct btf_enum64)
>> +            : vlen * sizeof (struct btf_enum);
>>         break;
>>       case BTF_KIND_FUNC_PROTO:
>> @@ -622,6 +624,15 @@ btf_asm_type (ctf_container_ref ctfc, 
>> ctf_dtdef_ref dtd)
>>         btf_size_type = 0;
>>       }
>> +  if (btf_kind == BTF_KIND_ENUM)
>> +    {
>> +      btf_kflag = dtd->dtd_enum_unsigned
>> +            ? BTF_KF_ENUM_UNSIGNED
>> +            : BTF_KF_ENUM_SIGNED;
>> +      if (dtd->dtd_data.ctti_size == 0x8)
>> +    btf_kind = BTF_KIND_ENUM64;
>> +   }
>> +
>>     dw2_asm_output_data (4, dtd->dtd_data.ctti_name, "btt_name");
>>     dw2_asm_output_data (4, BTF_TYPE_INFO (btf_kind, btf_kflag, 
>> btf_vlen),
>>                  "btt_info: kind=%u, kflag=%u, vlen=%u",
>> @@ -634,6 +645,7 @@ btf_asm_type (ctf_container_ref ctfc, 
>> ctf_dtdef_ref dtd)
>>       case BTF_KIND_UNION:
>>       case BTF_KIND_ENUM:
>>       case BTF_KIND_DATASEC:
>> +    case BTF_KIND_ENUM64:
>>         dw2_asm_output_data (4, dtd->dtd_data.ctti_size, "btt_size: %uB",
>>                  dtd->dtd_data.ctti_size);
>>         return;
>> @@ -707,13 +719,19 @@ btf_asm_sou_member (ctf_container_ref ctfc, 
>> ctf_dmdef_t * dmd)
>>       }
>>   }
>> -/* Asm'out an enum constant following a BTF_KIND_ENUM.  */
>> +/* Asm'out an enum constant following a BTF_KIND_ENUM{,64}.  */
>>   static void
>> -btf_asm_enum_const (ctf_dmdef_t * dmd)
>> +btf_asm_enum_const (unsigned int size, ctf_dmdef_t * dmd)
>>   {
>>     dw2_asm_output_data (4, dmd->dmd_name_offset, "bte_name");
>> -  dw2_asm_output_data (4, dmd->dmd_value, "bte_value");
>> +  if (size == 4)
>> +    dw2_asm_output_data (size, dmd->dmd_value, "bte_value");
>> +  else
>> +    {
>> +      dw2_asm_output_data (4, dmd->dmd_value & 0xffffffff, 
>> "bte_value_lo32");
>> +      dw2_asm_output_data (4, (dmd->dmd_value >> 32) & 0xffffffff, 
>> "bte_value_hi32");
>> +    }
>>   }
>>   /* Asm'out a function parameter description following a 
>> BTF_KIND_FUNC_PROTO.  */
>> @@ -871,7 +889,7 @@ output_asm_btf_sou_fields (ctf_container_ref ctfc, 
>> ctf_dtdef_ref dtd)
>>         btf_asm_sou_member (ctfc, dmd);
>>   }
>> -/* Output all enumerator constants following a BTF_KIND_ENUM.  */
>> +/* Output all enumerator constants following a BTF_KIND_ENUM{,64}.  */
>>   static void
>>   output_asm_btf_enum_list (ctf_container_ref ARG_UNUSED (ctfc),
>> @@ -881,7 +899,7 @@ output_asm_btf_enum_list (ctf_container_ref 
>> ARG_UNUSED (ctfc),
>>     for (dmd = dtd->dtd_u.dtu_members;
>>          dmd != NULL; dmd = (ctf_dmdef_t *) ctf_dmd_list_next (dmd))
>> -    btf_asm_enum_const (dmd);
>> +    btf_asm_enum_const (dtd->dtd_data.ctti_size, dmd);
>>   }
>>   /* Output all function arguments following a BTF_KIND_FUNC_PROTO.  */
>> diff --git a/gcc/ctfc.cc b/gcc/ctfc.cc
>> index 9773358a475..34030cbaa49 100644
>> --- a/gcc/ctfc.cc
>> +++ b/gcc/ctfc.cc
>> @@ -577,7 +577,7 @@ ctf_add_array (ctf_container_ref ctfc, uint32_t 
>> flag, const ctf_arinfo_t * arp,
>>   ctf_id_t
>>   ctf_add_enum (ctf_container_ref ctfc, uint32_t flag, const char * name,
>> -          HOST_WIDE_INT size, dw_die_ref die)
>> +          HOST_WIDE_INT size, bool eunsigned, dw_die_ref die)
>>   {
>>     ctf_dtdef_ref dtd;
>>     ctf_id_t type;
>> @@ -604,6 +604,7 @@ ctf_add_enum (ctf_container_ref ctfc, uint32_t 
>> flag, const char * name,
>>     gcc_assert (size <= CTF_MAX_SIZE);
>>     dtd->dtd_data.ctti_size = size;
>> +  dtd->dtd_enum_unsigned = eunsigned;
>>     ctfc->ctfc_num_stypes++;
>> @@ -630,10 +631,12 @@ ctf_add_enumerator (ctf_container_ref ctfc, 
>> ctf_id_t enid, const char * name,
>>     gcc_assert (kind == CTF_K_ENUM && vlen < CTF_MAX_VLEN);
>> -  /* Enum value is of type HOST_WIDE_INT in the compiler, dmd_value 
>> is int32_t
>> -     on the other hand.  Check bounds and skip adding this enum value 
>> if out of
>> -     bounds.  */
>> -  if ((value > INT_MAX) || (value < INT_MIN))
>> +  /* Enum value is of type HOST_WIDE_INT in the compiler, CTF 
>> enumerators
>> +     values in ctf_enum_t is limited to int32_t, BTF supports signed and
>> +     unsigned enumerators values of 32 and 64 bits, for both debug 
>> formats
>> +     we use ctf_dmdef_t.dmd_value entry of HOST_WIDE_INT type. So check
>> +     CTF bounds and skip adding this enum value if out of bounds.  */
>> +  if (!btf_debuginfo_p() && ((value > INT_MAX) || (value < INT_MIN)))
>>       {
>>         /* FIXME - Note this TBD_CTF_REPRESENTATION_LIMIT.  */
>>         return (1);
>> diff --git a/gcc/ctfc.h b/gcc/ctfc.h
>> index bcf3a43ae1b..48c381a008d 100644
>> --- a/gcc/ctfc.h
>> +++ b/gcc/ctfc.h
>> @@ -133,7 +133,7 @@ typedef struct GTY ((chain_next ("%h.dmd_next"))) 
>> ctf_dmdef
>>     ctf_id_t dmd_type;        /* Type of this member (for sou).  */
>>     uint32_t dmd_name_offset;    /* Offset of the name in str table.  */
>>     uint64_t dmd_offset;        /* Offset of this member in bits (for 
>> sou).  */
>> -  int dmd_value;        /* Value of this member (for enum).  */
>> +  HOST_WIDE_INT dmd_value;    /* Value of this member (for enum).  */
>>     struct ctf_dmdef * dmd_next;    /* A list node.  */
>>   } ctf_dmdef_t;
>> @@ -162,6 +162,7 @@ struct GTY ((for_user)) ctf_dtdef
>>     bool from_global_func; /* Whether this type was added from a global
>>                   function.  */
>>     uint32_t linkage;           /* Used in function types.  0=local, 
>> 1=global.  */
>> +  bool dtd_enum_unsigned;     /* Enum signedness.  */
>>     union GTY ((desc ("ctf_dtu_d_union_selector (&%1)")))
>>     {
>>       /* struct, union, or enum.  */
>> @@ -406,7 +407,7 @@ extern const char * ctf_add_string 
>> (ctf_container_ref, const char *,
>>   extern ctf_id_t ctf_add_reftype (ctf_container_ref, uint32_t, ctf_id_t,
>>                    uint32_t, dw_die_ref);
>>   extern ctf_id_t ctf_add_enum (ctf_container_ref, uint32_t, const 
>> char *,
>> -                  HOST_WIDE_INT, dw_die_ref);
>> +                  HOST_WIDE_INT, bool, dw_die_ref);
>>   extern ctf_id_t ctf_add_slice (ctf_container_ref, uint32_t, ctf_id_t,
>>                      uint32_t, uint32_t, dw_die_ref);
>>   extern ctf_id_t ctf_add_float (ctf_container_ref, uint32_t, const 
>> char *,
>> diff --git a/gcc/dwarf2ctf.cc b/gcc/dwarf2ctf.cc
>> index 397100004c2..748dd0cd8af 100644
>> --- a/gcc/dwarf2ctf.cc
>> +++ b/gcc/dwarf2ctf.cc
>> @@ -736,6 +736,7 @@ gen_ctf_enumeration_type (ctf_container_ref ctfc, 
>> dw_die_ref enumeration)
>>   {
>>     const char *enum_name = get_AT_string (enumeration, DW_AT_name);
>>     unsigned int bit_size = ctf_die_bitsize (enumeration);
>> +  unsigned int signedness = get_AT_unsigned (enumeration, 
>> DW_AT_encoding);
>>     int declaration_p = get_AT_flag (enumeration, DW_AT_declaration);
>>     ctf_id_t enumeration_type_id;
>> @@ -759,7 +760,9 @@ gen_ctf_enumeration_type (ctf_container_ref ctfc, 
>> dw_die_ref enumeration)
>>     /* Generate a CTF type for the enumeration.  */
>>     enumeration_type_id = ctf_add_enum (ctfc, CTF_ADD_ROOT,
>> -                      enum_name, bit_size / 8, enumeration);
>> +                      enum_name, bit_size / 8,
>> +                      (signedness == DW_ATE_unsigned),
>> +                      enumeration);
>>     /* Process the enumerators.  */
>>     {
>> diff --git a/gcc/testsuite/gcc.dg/debug/btf/btf-enum-1.c 
>> b/gcc/testsuite/gcc.dg/debug/btf/btf-enum-1.c
>> index 728493b0804..7e940529f1b 100644
>> --- a/gcc/testsuite/gcc.dg/debug/btf/btf-enum-1.c
>> +++ b/gcc/testsuite/gcc.dg/debug/btf/btf-enum-1.c
>> @@ -4,7 +4,7 @@
>>   /* { dg-options "-O0 -gbtf -dA" } */
>>   /* { dg-final { scan-assembler-times "\[\t \]0x6000004\[\t 
>> \]+\[^\n\]*btt_info" 1 } } */
>> -/* { dg-final { scan-assembler-times "\[\t \]0x6000003\[\t 
>> \]+\[^\n\]*btt_info" 1 } } */
>> +/* { dg-final { scan-assembler-times "\[\t \]0x86000003\[\t 
>> \]+\[^\n\]*btt_info" 1 } } */
>>   /* { dg-final { scan-assembler-times "ascii \"QAD.0\"\[\t 
>> \]+\[^\n\]*btf_string" 1 } } */
>>   /* { dg-final { scan-assembler-times "ascii \"QED.0\"\[\t 
>> \]+\[^\n\]*btf_string" 1 } } */
>>   /* { dg-final { scan-assembler-times "ascii \"QOD.0\"\[\t 
>> \]+\[^\n\]*btf_string" 1 } } */
>> diff --git a/gcc/testsuite/gcc.dg/debug/btf/btf-enum64-1.c 
>> b/gcc/testsuite/gcc.dg/debug/btf/btf-enum64-1.c
>> new file mode 100644
>> index 00000000000..e443d4c8c00
>> --- /dev/null
>> +++ b/gcc/testsuite/gcc.dg/debug/btf/btf-enum64-1.c
>> @@ -0,0 +1,44 @@
>> +/* Test BTF generation for 64 bits enums.  */
>> +
>> +/* { dg-do compile } */
>> +/* { dg-options "-O0 -gbtf -dA" } */
>> +
>> +/* { dg-final { scan-assembler-times "\[\t \].size\[\t \]myenum1,\[\t 
>> \]8" 1 } } */
>> +/* { dg-final { scan-assembler-times "\[\t \].size\[\t \]myenum2,\[\t 
>> \]8" 1 } } */
>> +/* { dg-final { scan-assembler-times "\[\t \].size\[\t \]myenum3,\[\t 
>> \]8" 1 } } */
>> +/* { dg-final { scan-assembler-times "\[\t \]0x13000003\[\t 
>> \]+\[^\n\]*btt_info" 2 } } */
>> +/* { dg-final { scan-assembler-times "\[\t \]0x93000003\[\t 
>> \]+\[^\n\]*btt_info" 1 } } */
>> +/* { dg-final { scan-assembler-times "\[\t \]0xffffffaa\[\t 
>> \]+\[^\n\]*bte_value_lo32" 2 } } */
>> +/* { dg-final { scan-assembler-times "\[\t \]0xff\[\t 
>> \]+\[^\n\]*bte_value_hi32" 3 } } */
>> +/* { dg-final { scan-assembler-times "ascii \"B1.0\"\[\t 
>> \]+\[^\n\]*btf_string" 1 } } */
>> +/* { dg-final { scan-assembler-times "ascii \"B2.0\"\[\t 
>> \]+\[^\n\]*btf_string" 1 } } */
>> +/* { dg-final { scan-assembler-times "ascii \"B3.0\"\[\t 
>> \]+\[^\n\]*btf_string" 1 } } */
>> +/* { dg-final { scan-assembler-times "ascii \"C1.0\"\[\t 
>> \]+\[^\n\]*btf_string" 1 } } */
>> +/* { dg-final { scan-assembler-times "ascii \"C2.0\"\[\t 
>> \]+\[^\n\]*btf_string" 1 } } */
>> +/* { dg-final { scan-assembler-times "ascii \"C3.0\"\[\t 
>> \]+\[^\n\]*btf_string" 1 } } */
>> +/* { dg-final { scan-assembler-times "ascii \"D1.0\"\[\t 
>> \]+\[^\n\]*btf_string" 1 } } */
>> +/* { dg-final { scan-assembler-times "ascii \"D2.0\"\[\t 
>> \]+\[^\n\]*btf_string" 1 } } */
>> +/* { dg-final { scan-assembler-times "ascii \"D3.0\"\[\t 
>> \]+\[^\n\]*btf_string" 1 } } */
>> +/* { dg-final { scan-assembler-times "bte_value_lo32" 9 } } */
>> +/* { dg-final { scan-assembler-times "bte_value_hi32" 9 } } */
>> +
>> +enum default_enum
>> +{
>> +  B1 = 0xffffffffaa,
>> +  B2 = 0xbbbbbbbb,
>> +  B3 = 0xaabbccdd,
>> +} myenum1 = B1;
>> +
>> +enum explicit_unsigned
>> +{
>> +  C1 = 0xffffffffbbUL,
>> +  C2 = 0xbbbbbbbb,
>> +  C3 = 0xaabbccdd,
>> +} myenum2 = C1;
>> +
>> +enum signed64
>> +{
>> +  D1 = 0xffffffffaa,
>> +  D2 = 0xbbbbbbbb,
>> +  D3 = -0x1,
>> +} myenum3 = D1;
>> diff --git a/include/btf.h b/include/btf.h
>> index 78b551ced23..eba67f9d599 100644
>> --- a/include/btf.h
>> +++ b/include/btf.h
>> @@ -109,7 +109,8 @@ struct btf_type
>>   #define BTF_KIND_VAR        14    /* Variable.  */
>>   #define BTF_KIND_DATASEC    15    /* Section such as .bss or .data.  */
>>   #define BTF_KIND_FLOAT        16    /* Floating point.  */
>> -#define BTF_KIND_MAX        BTF_KIND_FLOAT
>> +#define BTF_KIND_ENUM64     19    /* Enumeration up to 64 bits.  */
>> +#define BTF_KIND_MAX        BTF_KIND_ENUM64
>>   #define NR_BTF_KINDS        (BTF_KIND_MAX + 1)
>>   /* For some BTF_KINDs, struct btf_type is immediately followed by
>> @@ -130,14 +131,17 @@ struct btf_type
>>   #define BTF_INT_BOOL    (1 << 2)
>>   /* BTF_KIND_ENUM is followed by VLEN struct btf_enum entries,
>> -   which describe the enumerators. Note that BTF currently only
>> -   supports signed 32-bit enumerator values.  */
>> +   which describe the enumerators. */
>>   struct btf_enum
>>   {
>>     uint32_t name_off;    /* Offset in string section of enumerator 
>> name.  */
>>     int32_t  val;        /* Enumerator value.  */
>>   };
>> +/* BTF_KF_ENUM_ holds the flags for kflags in BTF_KIND_ENUM{,64}.  */
>> +#define BTF_KF_ENUM_UNSIGNED    (0)
>> +#define BTF_KF_ENUM_SIGNED     (1 << 0)
>> +
>>   /* BTF_KIND_ARRAY is followed by a single struct btf_array.  */
>>   struct btf_array
>>   {
>> @@ -190,6 +194,15 @@ struct btf_var_secinfo
>>     uint32_t size;    /* Size (in bytes) of variable.  */
>>   };
>> +/* BTF_KIND_ENUM64 is followed by VLEN struct btf_enum64 entries,
>> +   which describe the 64 bits enumerators.  */
>> +struct btf_enum64
>> +{
>> +  uint32_t name_off;    /* Offset in string section of enumerator 
>> name.  */
>> +  uint32_t val_lo32;    /* lower 32-bit value for a 64-bit value 
>> Enumerator */
>> +  uint32_t val_hi32;    /* high 32-bit value for a 64-bit value 
>> Enumerator */
>> +};
>> +
>>   #ifdef    __cplusplus
>>   }
>>   #endif
> 


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

end of thread, other threads:[~2022-10-31 18:26 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-08-29 21:11 [PATCH] btf: Add support to BTF_KIND_ENUM64 type Guillermo E. Martinez
2022-09-20 19:32 ` Guillermo E. Martinez
2022-09-28 18:45 ` David Faust
2022-09-28 21:07   ` Guillermo E. Martinez
2022-09-28 21:15 ` [PATCH v2] " Guillermo E. Martinez
2022-09-29 22:35   ` Indu Bhagat
2022-10-03 14:39     ` Guillermo E. Martinez
2022-10-11 18:55       ` Indu Bhagat
2022-10-15  3:45         ` Guillermo E. Martinez
2022-10-15  3:55   ` [PATCH v3] " Guillermo E. Martinez
2022-10-18 21:40     ` Indu Bhagat
2022-10-18 21:48       ` Guillermo E. Martinez
2022-10-20  2:05     ` [PATCH v4] " Guillermo E. Martinez
2022-10-21  9:28       ` Indu Bhagat
2022-10-31 18:26         ` Indu Bhagat

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).