Re: [PATCH] btf: improve -dA comments for testsuite

[Indu Bhagat via Gcc-patches]

On 5/30/23 09:08, David Faust wrote:
@@ -793,7 +917,8 @@ btf_asm_enum_const (unsigned int size, ctf_dmdef_t * dmd)
   /* Asm'out a function parameter description following a BTF_KIND_FUNC_PROTO. 
 */
   
   static void
-btf_asm_func_arg (ctf_func_arg_t * farg, size_t stroffset)
+btf_asm_func_arg (ctf_container_ref ctfc, ctf_func_arg_t * farg,
+                 size_t stroffset)
   {
     /* If the function arg does not have a name, refer to the null string at
        the start of the string table. This ensures correct encoding for varargs
@@ -803,31 +928,33 @@ btf_asm_func_arg (ctf_func_arg_t * farg, size_t stroffset)
     else
       dw2_asm_output_data (4, 0, "farg_name");
   
-  dw2_asm_output_data (4, (btf_removed_type_p (farg->farg_type)
-                          ? BTF_VOID_TYPEID
-                          : get_btf_id (farg->farg_type)),
-                      "farg_type");
+  btf_asm_type_ref ("farg_type", ctfc, (btf_removed_type_p (farg->farg_type)
+                                       ? BTF_VOID_TYPEID
+                                       : get_btf_id (farg->farg_type)));
   }
   
   /* Asm'out a BTF_KIND_FUNC type.  */
   
Lets keep the function level comments updated.  Apart from
btf_asm_func_type, this comment applies to other functions touched in
this commit, like btf_asm_datasec_type.
I don't follow. All those functions are still doing the same thing.
What needs to be updated exactly?

I meant updating the function-level comments for the additional args 
that are added.

I saw that in this patch, the new functions added follow the style of 
explaining the args, like,

+/* Return the relative index of the variable with final BTF ID ABS.  */
+
+static ctf_id_t
+btf_relative_var_id (ctf_id_t abs)

Hence, my comment.  But on second look, however, I see that the file 
keeps a mix of different styles. This one is up to you then. It makes 
sense to leave out the self-explanatory args.

   static void
-btf_asm_func_type (ctf_dtdef_ref dtd)
+btf_asm_func_type (ctf_container_ref ctfc, ctf_dtdef_ref dtd, size_t i)
   {
-  dw2_asm_output_data (4, dtd->dtd_data.ctti_name, "btt_name");
-  dw2_asm_output_data (4, BTF_TYPE_INFO (BTF_KIND_FUNC, 0,
-                                         dtd->linkage),
-                       "btt_info: kind=%u, kflag=%u, linkage=%u",
-                       BTF_KIND_FUNC, 0, dtd->linkage);
-  dw2_asm_output_data (4, get_btf_id (dtd->dtd_data.ctti_type), "btt_type");
+  ctf_id_t ref_id = dtd->dtd_data.ctti_type;
+  dw2_asm_output_data (4, dtd->dtd_data.ctti_name,
+                      "TYPE %lu BTF_KIND_FUNC '%s'",
+                      num_types_added + num_vars_added + 1 + i,
+                      dtd->dtd_name);
+  dw2_asm_output_data (4, BTF_TYPE_INFO (BTF_KIND_FUNC, 0, dtd->linkage),
+                      "btt_info: kind=%u, kflag=%u, linkage=%u",
+                      BTF_KIND_FUNC, 0, dtd->linkage);
+  btf_asm_type_ref ("btt_type", ctfc, get_btf_id (ref_id));
   }
   
   /* Asm'out a variable entry following a BTF_KIND_DATASEC.  */
   
   static void
-btf_asm_datasec_entry (struct btf_var_secinfo info)
+btf_asm_datasec_entry (ctf_container_ref ctfc, struct btf_var_secinfo info)
   {
-  dw2_asm_output_data (4, info.type, "bts_type");
+  btf_asm_type_ref ("bts_type", ctfc, info.type);
     dw2_asm_output_data (4, info.offset, "bts_offset");
     dw2_asm_output_data (4, info.size, "bts_size");
   }
@@ -835,9 +962,12 @@ btf_asm_datasec_entry (struct btf_var_secinfo info)
   /* Asm'out a whole BTF_KIND_DATASEC, including its variable entries.  */
   
   static void
-btf_asm_datasec_type (btf_datasec_t ds, size_t stroffset)
+btf_asm_datasec_type (ctf_container_ref ctfc, btf_datasec_t ds, ctf_id_t id,
+                     size_t stroffset)
   {
-  dw2_asm_output_data (4, ds.name_offset + stroffset, "btt_name");
+  dw2_asm_output_data (4, ds.name_offset + stroffset,
+                      "TYPE %lu BTF_KIND_DATASEC '%s'",
+                      btf_absolute_datasec_id (id), ds.name);
     dw2_asm_output_data (4, BTF_TYPE_INFO (BTF_KIND_DATASEC, 0,
                                         ds.entries.length ()),
                       "btt_info");
@@ -845,7 +975,7 @@ btf_asm_datasec_type (btf_datasec_t ds, size_t stroffset)
        loaders such as libbpf.  */
     dw2_asm_output_data (4, 0, "btt_size");
     for (size_t i = 0; i < ds.entries.length (); i++)
-    btf_asm_datasec_entry (ds.entries[i]);
+    btf_asm_datasec_entry (ctfc, ds.entries[i]);
   }