| Message ID | 55B7589F.7020400@arm.com |
|---|---|
| State | New |
| Headers | show |
On 28/07/15 11:25, Kyrill Tkachov wrote: > Hi all, > > Currently we assign the wrong rtx cost to instructions of the form > add x0, x0, x1, sxtw > > that is, an arith operation plus a single extend (no shifting). > We correctly catch the cases where the extend is inside a shift, but > not the simple case. > > This patch fixes that oversight by catching the simple case in > aarch64_rtx_arith_op_extract_p and thus making sure that it gets > assigned the alu.extend_arith extra cost. > > Bootstrapped and tested on aarch64. > > Ok for trunk? OK. R. > Thanks, > Kyrill > > > 2015-07-28 Kyrylo Tkachov <kyrylo.tkachov@arm.com> > > * config/aarch64/aarch64.c (aarch64_rtx_arith_op_extract_p): > Handle simple SIGN_EXTEND or ZERO_EXTEND. > (aarch64_rtx_costs): Properly strip extend or extract before > passing down to rtx costs again. > > > aarch64-arith-extend-costs.patch > > > commit 6ad208ea10b0893b356dab9d0c6f59821441229c > Author: Kyrylo Tkachov <kyrylo.tkachov@arm.com> > Date: Fri Jul 24 15:02:10 2015 +0100 > > [AArch64] Properly handle simple arith+extend ops in rtx costs > > diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c > index 617c079..eb70c30 100644 > --- a/gcc/config/aarch64/aarch64.c > +++ b/gcc/config/aarch64/aarch64.c > @@ -5622,6 +5622,11 @@ aarch64_rtx_arith_op_extract_p (rtx x, machine_mode mode) > return true; > } > } > + /* The simple case <ARITH>, XD, XN, XM, [us]xt. > + No shift. */ > + else if (GET_CODE (x) == SIGN_EXTEND > + || GET_CODE (x) == ZERO_EXTEND) > + return REG_P (XEXP (x, 0)); > > return false; > } > @@ -6133,7 +6138,8 @@ cost_minus: > if (speed) > *cost += extra_cost->alu.extend_arith; > > - *cost += rtx_cost (XEXP (XEXP (op1, 0), 0), VOIDmode, > + op1 = aarch64_strip_extend (op1); > + *cost += rtx_cost (op1, VOIDmode, > (enum rtx_code) GET_CODE (op1), 0, speed); > return true; > } > @@ -6211,7 +6217,8 @@ cost_plus: > if (speed) > *cost += extra_cost->alu.extend_arith; > > - *cost += rtx_cost (XEXP (XEXP (op0, 0), 0), VOIDmode, > + op0 = aarch64_strip_extend (op0); > + *cost += rtx_cost (op0, VOIDmode, > (enum rtx_code) GET_CODE (op0), 0, speed); > return true; > } >
> On Jul 28, 2015, at 3:25 AM, Kyrill Tkachov <kyrylo.tkachov@arm.com> wrote: > > Hi all, > > Currently we assign the wrong rtx cost to instructions of the form > add x0, x0, x1, sxtw > > that is, an arith operation plus a single extend (no shifting). > We correctly catch the cases where the extend is inside a shift, but > not the simple case. > > This patch fixes that oversight by catching the simple case in > aarch64_rtx_arith_op_extract_p and thus making sure that it gets > assigned the alu.extend_arith extra cost. This patch reminds me, on thunderx the cost for add with sign extend is different from add with zero extend. The zero extend case is the same as a normal add while sign extend is one extra cycle. So soon we need to split extend to zextend and sextend. Thanks, Andrew > > Bootstrapped and tested on aarch64. > > Ok for trunk? > Thanks, > Kyrill > > > 2015-07-28 Kyrylo Tkachov <kyrylo.tkachov@arm.com> > > * config/aarch64/aarch64.c (aarch64_rtx_arith_op_extract_p): > Handle simple SIGN_EXTEND or ZERO_EXTEND. > (aarch64_rtx_costs): Properly strip extend or extract before > passing down to rtx costs again. > <aarch64-arith-extend-costs.patch>
commit 6ad208ea10b0893b356dab9d0c6f59821441229c
Author: Kyrylo Tkachov <kyrylo.tkachov@arm.com>
Date: Fri Jul 24 15:02:10 2015 +0100
[AArch64] Properly handle simple arith+extend ops in rtx costs
diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index 617c079..eb70c30 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -5622,6 +5622,11 @@ aarch64_rtx_arith_op_extract_p (rtx x, machine_mode mode)
return true;
}
}
+ /* The simple case <ARITH>, XD, XN, XM, [us]xt.
+ No shift. */
+ else if (GET_CODE (x) == SIGN_EXTEND
+ || GET_CODE (x) == ZERO_EXTEND)
+ return REG_P (XEXP (x, 0));
return false;
}
@@ -6133,7 +6138,8 @@ cost_minus:
if (speed)
*cost += extra_cost->alu.extend_arith;
- *cost += rtx_cost (XEXP (XEXP (op1, 0), 0), VOIDmode,
+ op1 = aarch64_strip_extend (op1);
+ *cost += rtx_cost (op1, VOIDmode,
(enum rtx_code) GET_CODE (op1), 0, speed);
return true;
}
@@ -6211,7 +6217,8 @@ cost_plus:
if (speed)
*cost += extra_cost->alu.extend_arith;
- *cost += rtx_cost (XEXP (XEXP (op0, 0), 0), VOIDmode,
+ op0 = aarch64_strip_extend (op0);
+ *cost += rtx_cost (op0, VOIDmode,
(enum rtx_code) GET_CODE (op0), 0, speed);
return true;
}
Hi all, Currently we assign the wrong rtx cost to instructions of the form add x0, x0, x1, sxtw that is, an arith operation plus a single extend (no shifting). We correctly catch the cases where the extend is inside a shift, but not the simple case. This patch fixes that oversight by catching the simple case in aarch64_rtx_arith_op_extract_p and thus making sure that it gets assigned the alu.extend_arith extra cost. Bootstrapped and tested on aarch64. Ok for trunk? Thanks, Kyrill 2015-07-28 Kyrylo Tkachov <kyrylo.tkachov@arm.com> * config/aarch64/aarch64.c (aarch64_rtx_arith_op_extract_p): Handle simple SIGN_EXTEND or ZERO_EXTEND. (aarch64_rtx_costs): Properly strip extend or extract before passing down to rtx costs again.