[PATCH] combine: Don't record for UNDO_MODE pointers into regno_reg_rtx array [PR104985]

[Jakub Jelinek via Gcc-patches]

Hi!

The testcase in the PR fails under valgrind on mips64 (but only Martin
can reproduce, I couldn't).
But the problem reported there is that SUBST_MODE remembers addresses
into the regno_reg_rtx array, then some splitter needs a new pseudo
and calls gen_reg_rtx, which reallocates the regno_reg_rtx array
and finally undo operation is done and dereferences the old regno_reg_rtx
entry.
The rtx values stored in regno_reg_rtx array seems to be created
by gen_reg_rtx only and since then aren't modified, all we do for it
is adjusting its fields (e.g. adjust_reg_mode that SUBST_MODE does).

So, I think it is useless to use where.r for UNDO_MODE and store
&regno_reg_rtx[regno] in struct undo, we can store just
regno_reg_rtx[regno] (i.e. pointer to the REG itself instead of
pointer to pointer to REG) or could also store just the regno.

The following patch does the former.

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

Or there are variant patches in the PR, either a minimal version of
this patch, or one that records regno and adjusts all SUBST_MODE uses.
Also, not sure I like very much a function name in all caps, but I wanted
to preserve the users and all other SUBST and SUBST_* are also all caps.
Yet another possibility would be keep do_SUBST_MODE with the new
arguments and
#define SUBST_MODE(INTO, NEWVAL)  do_SUBST_MODE ((INTO), (NEWVAL))

2022-03-29  Jakub Jelinek  <ja...@redhat.com>

        PR rtl-optimization/104985
        * combine.cc (struct undo): Add where.m member.
        (do_SUBST_MODE): Rename to ...
        (SUBST_MODE): ... this.  No longer a macro.  Change first argument
        from rtx * into rtx, operate on INTO rather than *INTO and save
        INTO into where.m.
        (try_combine, undo_to_marker): For UNDO_MODE, use undo->where.m
        instead of *undo->where.r.

--- gcc/combine.cc.jj   2022-02-16 14:48:23.847681860 +0100
+++ gcc/combine.cc      2022-03-28 14:37:04.708490651 +0200
@@ -382,7 +382,7 @@ struct undo
   struct undo *next;
   enum undo_kind kind;
   union { rtx r; int i; machine_mode m; struct insn_link *l; } old_contents;
-  union { rtx *r; int *i; struct insn_link **l; } where;
+  union { rtx *r; int *i; rtx m; struct insn_link **l; } where;
 };
 
 /* Record a bunch of changes to be undone, up to MAX_UNDO of them.
@@ -761,10 +761,10 @@ do_SUBST_INT (int *into, int newval)
    well.  */
 
 static void
-do_SUBST_MODE (rtx *into, machine_mode newval)
+SUBST_MODE (rtx into, machine_mode newval)
 {
   struct undo *buf;
-  machine_mode oldval = GET_MODE (*into);
+  machine_mode oldval = GET_MODE (into);
 
   if (oldval == newval)
     return;
@@ -775,15 +775,13 @@ do_SUBST_MODE (rtx *into, machine_mode n
     buf = XNEW (struct undo);
 
   buf->kind = UNDO_MODE;
-  buf->where.r = into;
+  buf->where.m = into;
   buf->old_contents.m = oldval;
-  adjust_reg_mode (*into, newval);
+  adjust_reg_mode (into, newval);
 
   buf->next = undobuf.undos, undobuf.undos = buf;
 }
 
-#define SUBST_MODE(INTO, NEWVAL)  do_SUBST_MODE (&(INTO), (NEWVAL))
-
 /* Similar to SUBST, but NEWVAL is a LOG_LINKS expression.  */
 
 static void
@@ -4082,7 +4080,7 @@ try_combine (rtx_insn *i3, rtx_insn *i2,
       for (undo = undobuf.undos; undo; undo = undo->next)
        if (undo->kind == UNDO_MODE)
          {
-           rtx reg = *undo->where.r;
+           rtx reg = undo->where.m;
            machine_mode new_mode = GET_MODE (reg);
            machine_mode old_mode = undo->old_contents.m;
 
@@ -4755,7 +4753,7 @@ undo_to_marker (void *marker)
          *undo->where.i = undo->old_contents.i;
          break;
        case UNDO_MODE:
-         adjust_reg_mode (*undo->where.r, undo->old_contents.m);
+         adjust_reg_mode (undo->where.m, undo->old_contents.m);
          break;
        case UNDO_LINKS:
          *undo->where.l = undo->old_contents.l;

        Jakub