From 2f12ce94166f411e4b9084b1c89738bb480343cc Mon Sep 17 00:00:00 2001
From: Thomas Schwinge <thomas@codesourcery.com>
Date: Tue, 4 Jul 2023 11:46:00 +0200
Subject: [PATCH] GGC, GTY: No pointer walking for 'atomic' in PCH
 'gt_pch_note_object'

Since it's inception in 2011 commit 555c3771903cc461949f06acf28f92fc067b6a1c
(Subversion r173996) "New GTY ((atomic)) option", the following rationale has
been given in (nowadays) 'gcc/gengtype.cc':

    If a pointer type is marked as "atomic", we process the
    field itself, but we don't walk the data that they point to.

    There are two main cases where we walk types: to mark
    pointers that are reachable, and to relocate pointers when
    writing a PCH file.  In both cases, an atomic pointer is
    itself marked or relocated, but the memory that it points
    to is left untouched.  In the case of PCH, that memory will
    be read/written unchanged to the PCH file.

Therefore, we may completely skip the boilerplate pointer walking, which we
didn't for PCH 'gt_pch_note_object'.

    --- build-gcc/gcc/gt-c-c-decl.h	2023-06-26 08:59:55.120395571 +0200
    +++ build-gcc/gcc/gt-c-c-decl.h	2023-07-05 15:58:36.286165439 +0200
    @@ -1138,7 +1138,7 @@
                 case TS_OPTIMIZATION:
                   gt_pch_n_15cl_optimization ((*x).generic.optimization.opts);
                   if ((*x).generic.optimization.optabs != NULL) {
    -                gt_pch_note_object ((*x).generic.optimization.optabs, x, gt_pch_p_14lang_tree_node);
    +                gt_pch_note_object ((*x).generic.optimization.optabs, x, NULL);
                   }
                   break;
                 case TS_TARGET_OPTION:
    --- build-gcc/gcc/gt-cp-tree.h	2023-06-26 08:59:55.120395571 +0200
    +++ build-gcc/gcc/gt-cp-tree.h	2023-07-05 15:58:36.286165439 +0200
    @@ -1452,7 +1452,7 @@
                 case TS_OPTIMIZATION:
                   gt_pch_n_15cl_optimization ((*x).generic.optimization.opts);
                   if ((*x).generic.optimization.optabs != NULL) {
    -                gt_pch_note_object ((*x).generic.optimization.optabs, x, gt_pch_p_14lang_tree_node);
    +                gt_pch_note_object ((*x).generic.optimization.optabs, x, NULL);
                   }
                   break;
                 case TS_TARGET_OPTION:
    [...]

..., which is for 'gcc/tree-core.h':

    struct GTY(()) tree_optimization_option {
      struct tree_base base;
      [...]
      struct cl_optimization *opts;
      [...]
      void *GTY ((atomic)) optabs;
      [...]
      struct target_optabs *GTY ((skip)) base_optabs;
    };

..., which means we'll still 'gt_pch_note_object' 'optabs' itself.  (That's
important; if we skip those completely, we'll later fail the
'gcc_assert (result)' in 'gcc/ggc-common.cc:relocate_ptrs'.)  However, we no
longer attempt to walk 'optabs' via 'gt_pch_save', which per the original
'build-gcc/gcc/gt-c-c-decl.h:gt_pch_p_14lang_tree_node' call here meant:

    void
    gt_pch_p_14lang_tree_node (ATTRIBUTE_UNUSED void *this_obj,
    	void *x_p,
    	ATTRIBUTE_UNUSED gt_pointer_operator op,
    	ATTRIBUTE_UNUSED void *cookie)
    {
      union lang_tree_node * x ATTRIBUTE_UNUSED = (union lang_tree_node *)x_p;
      switch ((int) (TREE_CODE (&((*x)).generic) == IDENTIFIER_NODE))
        {
        case 0:
          switch ((int) (tree_node_structure (&((*x).generic))))
            {
    [...]
            case TS_OPTIMIZATION:
              if ((void *)(x) == this_obj)
                op (&((*x).generic.optimization.opts), NULL, cookie);
              if ((*x).generic.optimization.optabs != NULL) {
                if ((void *)(x) == this_obj)
                  op (&((*x).generic.optimization.optabs), NULL, cookie);
              }
              break;
    [...]
          break;
    [...]
        }
    }

Given that we're changing the
'gt_pch_note_object ((*x).generic.optimization.optabs, x, [...])' call, in
'gt_pch_p_14lang_tree_node' it was the case that (the second) '(x) == this_obj'
did *not* hold, and we therefore didn't call
'op (&((*x).generic.optimization.optabs), NULL, cookie)', and therefore not
invoking 'gt_pch_p_14lang_tree_node' at all does achieve the same thing (no-op)
-- per my understanding of all this machinery.

The other (few) changes/instances of 'atomic' behave similarly.

As a corollary, we may then tag 'const' data as 'atomic' (for example:
'const unsigned char * GTY((atomic)) mode_table'), without running into 'const'
mismatches, 'error: invalid conversion':

    [...]
    gtype-desc.cc: In function 'void gt_pch_nx_lto_file_decl_data(void*)':
    gtype-desc.cc:6531:34: error: invalid conversion from 'const void*' to 'void*' [-fpermissive]
             gt_pch_note_object ((*x).mode_table, x, gt_pch_p_18lto_file_decl_data);
                                      ^
    In file included from [...]/source-gcc/gcc/hash-table.h:247:0,
                     from [...]/source-gcc/gcc/coretypes.h:486,
                     from gtype-desc.cc:23:
    [...]/source-gcc/gcc/ggc.h:47:12: note:   initializing argument 1 of 'int gt_pch_note_object(void*, void*, gt_note_pointers, size_t)'
     extern int gt_pch_note_object (void *, void *, gt_note_pointers,
                ^
    make[2]: *** [Makefile:1180: gtype-desc.o] Error 1
    [...]

..., which was the original motivation of this exercise.

	gcc/
	* gengtype.cc (struct walk_type_data): Add 'bool atomic_p'.
	(walk_type, write_types_process_field): Handle it.
	* ggc-common.cc (gt_pch_note_reorder, gt_pch_save): Likewise.
---
 gcc/gengtype.cc   | 10 +++++++++-
 gcc/ggc-common.cc |  4 ++--
 2 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/gcc/gengtype.cc b/gcc/gengtype.cc
index 49ddba684af..07c7c4e91c8 100644
--- a/gcc/gengtype.cc
+++ b/gcc/gengtype.cc
@@ -2451,6 +2451,7 @@ struct walk_type_data
   bool in_ptr_field;
   bool have_this_obj;
   bool in_nested_ptr;
+  bool atomic_p;
 };
 
 
@@ -2747,11 +2748,16 @@ walk_type (type_p t, struct walk_type_data *d)
 	   be read/written unchanged to the PCH file.  */
 	if (atomic_p)
 	  {
+	    bool old_atomic_p = d->atomic_p;
+	    d->atomic_p = true;
+
 	    oprintf (d->of, "%*sif (%s != NULL) {\n", d->indent, "", d->val);
 	    d->indent += 2;
 	    d->process_field (t, d);
 	    d->indent -= 2;
 	    oprintf (d->of, "%*s}\n", d->indent, "");
+
+	    d->atomic_p = old_atomic_p;
 	    break;
 	  }
 
@@ -3214,7 +3220,9 @@ write_types_process_field (type_p f, const struct walk_type_data *d)
 	    oprintf (d->of, ", x");
 	  else
 	    oprintf (d->of, ", %s", d->prev_val[3]);
-	  if (d->orig_s)
+	  if (d->atomic_p)
+	    oprintf (d->of, ", NULL");
+	  else if (d->orig_s)
 	    {
 	      oprintf (d->of, ", gt_%s_", wtd->param_prefix);
 	      output_mangled_typename (d->of, d->orig_s);
diff --git a/gcc/ggc-common.cc b/gcc/ggc-common.cc
index bed7a9d4d02..f30ac748688 100644
--- a/gcc/ggc-common.cc
+++ b/gcc/ggc-common.cc
@@ -316,7 +316,8 @@ gt_pch_note_reorder (void *obj, void *note_ptr_cookie,
   gcc_assert (data && data->note_ptr_cookie == note_ptr_cookie);
   /* The GTY 'reorder' option doesn't make sense if we don't walk pointers,
      such as for strings.  */
-  gcc_checking_assert (data->note_ptr_fn != gt_pch_p_S);
+  gcc_checking_assert (data->note_ptr_fn != NULL
+		       && data->note_ptr_fn != gt_pch_p_S);
 
   data->reorder_fn = reorder_fn;
 }
@@ -640,7 +641,6 @@ gt_pch_save (FILE *f)
 				   state.ptrs[i]->note_ptr_cookie,
 				   relocate_ptrs, &state);
       gt_note_pointers note_ptr_fn = state.ptrs[i]->note_ptr_fn;
-      gcc_checking_assert (note_ptr_fn != NULL);
       /* 'gt_pch_p_S' enables certain special handling, but otherwise
      corresponds to no 'note_ptr_fn'.  */
       if (note_ptr_fn == gt_pch_p_S)
-- 
2.34.1

