On Tue, Apr 05, 2022 at 04:56:55PM -0500, Segher Boessenkool wrote:
> > 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.
>
> I like this one best I think :-)
>
> > 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))
>
> Like the other things in combine.c do, so please change to that? Which
> means changing less than you do now :-)
>
> Changing this all to not have macros is better of course, and can be
> tastefully done even with C++ (it would use C++ features even :-) ), but
> let's do that all at once, and in stage 1.
>
> > - union { rtx *r; int *i; struct insn_link **l; } where;
> > + union { rtx *r; int *i; rtx m; struct insn_link **l; } where;
>
> NAK. It is not clear at all what "rtx m" means, esp. since there is an
> "rtx *r" already. In the PR you said "machine_mode m", that is clear of
> course, can you do that instead?
So in that case something like this (i.e. the regno variant, renamed
to subst_mode from SUBST_MODE, and naming the union member regno rather
than m)?
And for GCC 13 change do_SUBST_INT to subst_int with int &into arg,
do_SUBST_LINK to subst_link with struct insn_link *&into arg,
and do_SUBST into subst with rtx &into arg?
2022-04-06 Jakub Jelinek <ja...@redhat.com>
PR rtl-optimization/104985
* combine.cc (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.
--- gcc/combine.cc.jj 2022-03-30 09:11:42.331261823 +0200
+++ gcc/combine.cc 2022-04-06 10:29:55.333106447 +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; int regno; struct insn_link **l; } where;
};
/* Record a bunch of changes to be undone, up to MAX_UNDO of them.
@@ -761,10 +761,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;
@@ -775,15 +776,13 @@ do_SUBST_MODE (rtx *into, machine_mode n
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
@@ -3186,7 +3185,7 @@ try_combine (rtx_insn *i3, rtx_insn *i2,
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];
}
}
@@ -3576,7 +3575,7 @@ try_combine (rtx_insn *i3, rtx_insn *i2,
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)];
}
@@ -3712,7 +3711,7 @@ try_combine (rtx_insn *i3, rtx_insn *i2,
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)];
}
}
@@ -4082,7 +4081,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 = regno_reg_rtx[undo->where.regno];
machine_mode new_mode = GET_MODE (reg);
machine_mode old_mode = undo->old_contents.m;
@@ -4755,7 +4754,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;
@@ -6819,7 +6819,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];
}
Jakub
