public inbox for gcc-cvs@sourceware.org
help / color / mirror / Atom feed
From: Jakub Jelinek <jakub@gcc.gnu.org>
To: gcc-cvs@gcc.gnu.org
Subject: [gcc r11-9845] combine: Don't record for UNDO_MODE pointers into regno_reg_rtx array [PR104985]
Date: Wed, 13 Apr 2022 04:28:11 +0000 (GMT)	[thread overview]
Message-ID: <20220413042811.09DBC3858D28@sourceware.org> (raw)

https://gcc.gnu.org/g:a487dbd802d71bcea8feaa41defde92a6848d0c6

commit r11-9845-ga487dbd802d71bcea8feaa41defde92a6848d0c6
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Wed Apr 6 18:42:52 2022 +0200

    combine: Don't record for UNDO_MODE pointers into regno_reg_rtx array [PR104985]
    
    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 latter, and because SUBST_MODE no longer
    needs to be a macro, changes all SUBST_MODE uses to subst_mode.
    
    2022-04-06  Jakub Jelinek  <jakub@redhat.com>
    
            PR rtl-optimization/104985
            * combine.c (struct undo): Add where.regno member.
            (do_SUBST_MODE): Rename to ...
            (subst_mode): ... this.  Change first argument from rtx * into int,
            operate on regno_reg_rtx[regno] and save regno into where.regno.
            (SUBST_MODE): Remove.
            (try_combine): Use subst_mode instead of SUBST_MODE, change first
            argument from regno_reg_rtx[whatever] to whatever.  For UNDO_MODE, use
            regno_reg_rtx[undo->where.regno] instead of *undo->where.r.
            (undo_to_marker): For UNDO_MODE, use regno_reg_rtx[undo->where.regno]
            instead of *undo->where.r.
            (simplify_set): Use subst_mode instead of SUBST_MODE, change first
            argument from regno_reg_rtx[whatever] to whatever.
    
    (cherry picked from commit 61bee6aed26eb30b798c75b9a595c9d51e080442)

Diff:
---
 gcc/combine.c | 26 +++++++++++++-------------
 1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/gcc/combine.c b/gcc/combine.c
index 58e55321897..5422852fd7f 100644
--- a/gcc/combine.c
+++ b/gcc/combine.c
@@ -396,7 +396,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; int regno; struct insn_link **l; } where;
 };
 
 /* Record a bunch of changes to be undone, up to MAX_UNDO of them.
@@ -792,10 +792,11 @@ do_SUBST_INT (int *into, int newval)
    well.  */
 
 static void
-do_SUBST_MODE (rtx *into, machine_mode newval)
+subst_mode (int regno, machine_mode newval)
 {
   struct undo *buf;
-  machine_mode oldval = GET_MODE (*into);
+  rtx reg = regno_reg_rtx[regno];
+  machine_mode oldval = GET_MODE (reg);
 
   if (oldval == newval)
     return;
@@ -806,15 +807,13 @@ do_SUBST_MODE (rtx *into, machine_mode newval)
     buf = XNEW (struct undo);
 
   buf->kind = UNDO_MODE;
-  buf->where.r = into;
+  buf->where.regno = regno;
   buf->old_contents.m = oldval;
-  adjust_reg_mode (*into, newval);
+  adjust_reg_mode (reg, 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
@@ -3301,7 +3300,7 @@ try_combine (rtx_insn *i3, rtx_insn *i2, rtx_insn *i1, rtx_insn *i0,
 		    newpat_dest = gen_rtx_REG (compare_mode, regno);
 		  else
 		    {
-		      SUBST_MODE (regno_reg_rtx[regno], compare_mode);
+		      subst_mode (regno, compare_mode);
 		      newpat_dest = regno_reg_rtx[regno];
 		    }
 		}
@@ -3691,7 +3690,7 @@ try_combine (rtx_insn *i3, rtx_insn *i2, rtx_insn *i1, rtx_insn *i0,
 		ni2dest = gen_rtx_REG (new_mode, REGNO (i2dest));
 	      else
 		{
-		  SUBST_MODE (regno_reg_rtx[REGNO (i2dest)], new_mode);
+		  subst_mode (REGNO (i2dest), new_mode);
 		  ni2dest = regno_reg_rtx[REGNO (i2dest)];
 		}
 
@@ -3828,7 +3827,7 @@ try_combine (rtx_insn *i3, rtx_insn *i2, rtx_insn *i1, rtx_insn *i0,
 		newdest = gen_rtx_REG (split_mode, REGNO (i2dest));
 	      else
 		{
-		  SUBST_MODE (regno_reg_rtx[REGNO (i2dest)], split_mode);
+		  subst_mode (REGNO (i2dest), split_mode);
 		  newdest = regno_reg_rtx[REGNO (i2dest)];
 		}
 	    }
@@ -4196,7 +4195,7 @@ try_combine (rtx_insn *i3, rtx_insn *i2, rtx_insn *i1, rtx_insn *i0,
       for (undo = undobuf.undos; undo; undo = undo->next)
 	if (undo->kind == UNDO_MODE)
 	  {
-	    rtx reg = *undo->where.r;
+	    rtx reg = regno_reg_rtx[undo->where.regno];
 	    machine_mode new_mode = GET_MODE (reg);
 	    machine_mode old_mode = undo->old_contents.m;
 
@@ -4869,7 +4868,8 @@ 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 (regno_reg_rtx[undo->where.regno],
+			   undo->old_contents.m);
 	  break;
 	case UNDO_LINKS:
 	  *undo->where.l = undo->old_contents.l;
@@ -6944,7 +6944,7 @@ simplify_set (rtx x)
 		new_dest = gen_rtx_REG (compare_mode, regno);
 	      else
 		{
-		  SUBST_MODE (regno_reg_rtx[regno], compare_mode);
+		  subst_mode (regno, compare_mode);
 		  new_dest = regno_reg_rtx[regno];
 		}


                 reply	other threads:[~2022-04-13  4:28 UTC|newest]

Thread overview: [no followups] expand[flat|nested]  mbox.gz  Atom feed

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220413042811.09DBC3858D28@sourceware.org \
    --to=jakub@gcc.gnu.org \
    --cc=gcc-cvs@gcc.gnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).