On 26/10/2020 12:43, Segher Boessenkool wrote:
> On Mon, Oct 26, 2020 at 01:28:42PM +0000, Alex Coplan wrote:
> > On 26/10/2020 07:12, Segher Boessenkool wrote:
> > > On Thu, Oct 15, 2020 at 09:59:24AM +0100, Alex Coplan wrote:
> > > Can you instead replace the mult by a shift somewhere earlier in
> > > make_extract? That would make a lot more sense :-)
> >
> > I guess we could do this, the only complication being that we can't
> > unconditionally rewrite the expression using a shift, since mult is
> > canonical
> > inside a mem (which is why we see it in the testcase in the PR).
>
> You can do it just inside the block you are already editing.
>
> > So if we did this, we'd have to remember that we did it earlier on, and
> > rewrite
> > it back to a mult accordingly.
>
> Yes, this function has ridiculously complicated cpontrol flow. So I
> cannot trick you into improving it? ;-)
>
> > Would you still like to see a version of the patch that does that, or is
> > this
> > version OK:
> > https://gcc.gnu.org/pipermail/gcc-patches/2020-October/557050.html ?
>
> I do not like handling both mult and ashift in one case like this, it
> complicates things for no good reason. Write it as two cases, and it
> should be good.
OK, the attached patch rewrites (mult x 2^n) to (ashift x n) at the top
of make_extraction so that the existing ASHIFT block can do the work for
us. We remember if we did it and then convert it back if necessary.
I'm not convinced that it's an improvement. What do you think?
Bootstrap/regtest in progress on aarch64-none-linux-gnu. I'll test other
platforms (as well as testing on top of 1/2) and repost with a proper
commit message if you think it looks good.
Alex
---
gcc/ChangeLog:
(make_extraction): Temporarily rewrite (mult x 2^n) so that we
can handle it as (ashift x n) and avoid emitting an extract where
extend+shift will suffice.
diff --git a/gcc/combine.c b/gcc/combine.c
index 4782e1d9dcc..991dc5eabf7 100644
--- a/gcc/combine.c
+++ b/gcc/combine.c
@@ -7628,10 +7628,25 @@ make_extraction (machine_mode mode, rtx inner,
HOST_WIDE_INT pos,
rtx new_rtx = 0;
rtx orig_pos_rtx = pos_rtx;
HOST_WIDE_INT orig_pos;
+ bool rewrote_mult_p = false;
if (pos_rtx && CONST_INT_P (pos_rtx))
pos = INTVAL (pos_rtx), pos_rtx = 0;
+ if (GET_CODE (inner) == MULT && CONST_INT_P (XEXP (inner, 1)))
+ {
+ /* We have to handle shifts disguised as multiplications
+ by powers of two since this is the canonical form for
+ mem addresses. */
+ const int shift_amt = exact_log2 (INTVAL (XEXP (inner, 1)));
+ if (shift_amt > 0)
+ {
+ PUT_CODE (inner, ASHIFT);
+ INTVAL (XEXP (inner, 1)) = shift_amt;
+ rewrote_mult_p = true;
+ }
+ }
+
if (GET_CODE (inner) == SUBREG
&& subreg_lowpart_p (inner)
&& (paradoxical_subreg_p (inner)
@@ -7663,7 +7678,7 @@ make_extraction (machine_mode mode, rtx inner,
HOST_WIDE_INT pos,
0, 0, len - INTVAL (XEXP (inner, 1)),
unsignedp, in_dest, in_compare);
if (new_rtx != 0)
- return gen_rtx_ASHIFT (mode, new_rtx, XEXP (inner, 1));
+ new_rtx = gen_rtx_ASHIFT (mode, new_rtx, XEXP (inner, 1));
}
else if (GET_CODE (inner) == TRUNCATE
/* If trying or potentionally trying to extract
@@ -7673,6 +7688,17 @@ make_extraction (machine_mode mode, rtx inner,
HOST_WIDE_INT pos,
&& known_le (pos + len, GET_MODE_PRECISION (is_mode)))
inner = XEXP (inner, 0);
+ if (rewrote_mult_p)
+ {
+ /* If we rewrote MULT -> ASHIFT, convert it back now. */
+ rtx x = new_rtx ? new_rtx : inner;
+ PUT_CODE (x, MULT);
+ INTVAL (XEXP (x, 1)) = 1 << INTVAL (XEXP (x, 1));
+ }
+
+ if (new_rtx)
+ return new_rtx;
+
inner_mode = GET_MODE (inner);
/* See if this can be done without an extraction. We never can if the
