| Message ID | 2ea52095880beff41ff1e53c7ebb6d7c6c35626c.1506971600.git.segher@kernel.crashing.org |
|---|---|
| State | New |
| Headers | show |
| Series | simplify-rtx: Remove non-simplifying simplification (PR77729) | expand |
On 10/02/2017 01:35 PM, Segher Boessenkool wrote: > If we have (X&C1)|C2 simplify_binary_operation_1 makes C1 as small as > possible. This makes worse code in common cases like when the AND with > C1 is from a zero-extension. This patch fixes it by removing this > transformation (twice). > > I tested this on 31 targets, also some variations that do the > transformation only for some conditions (like, do not do it if the > resulting constant looks "worse", e.g. has more stretches of ones). > 22 of those targets show no difference; 8 are best with this patch > variant (never do the transformation); and 64-bit hppa is not best, > but the difference is only four insns in a million. > > Bootstrapped and tested on powerpc64-linux {-m32,-m64}. Is this okay > for trunk? And backports? > > > Segher > > > 2017-10-02 Segher Boessenkool <segher@kernel.crashing.org> > > PR rtl-optimization/77729 > * simplify-rtx.c (simplify_binary_operation_1): Delete the (X&C1)|C2 > to (X&(C1&~C2))|C2 transformations. OK for the trunk. I'm not sure if the BZ in question qualifies this patch for backporting though. Release managers have the final say on the backport question. And FWIW, the PA being an outlier on an optimization question should never IMHO be a blocker :-) jeff
On Tue, Oct 3, 2017 at 12:40 AM, Jeff Law <law@redhat.com> wrote: > On 10/02/2017 01:35 PM, Segher Boessenkool wrote: >> If we have (X&C1)|C2 simplify_binary_operation_1 makes C1 as small as >> possible. This makes worse code in common cases like when the AND with >> C1 is from a zero-extension. This patch fixes it by removing this >> transformation (twice). >> >> I tested this on 31 targets, also some variations that do the >> transformation only for some conditions (like, do not do it if the >> resulting constant looks "worse", e.g. has more stretches of ones). >> 22 of those targets show no difference; 8 are best with this patch >> variant (never do the transformation); and 64-bit hppa is not best, >> but the difference is only four insns in a million. >> >> Bootstrapped and tested on powerpc64-linux {-m32,-m64}. Is this okay >> for trunk? And backports? >> >> >> Segher >> >> >> 2017-10-02 Segher Boessenkool <segher@kernel.crashing.org> >> >> PR rtl-optimization/77729 >> * simplify-rtx.c (simplify_binary_operation_1): Delete the (X&C1)|C2 >> to (X&(C1&~C2))|C2 transformations. > OK for the trunk. I'm not sure if the BZ in question qualifies this > patch for backporting though. Release managers have the final say on > the backport question. We usually avoid doing "optimization regression" backports unless very very important and obvious. Richard. > And FWIW, the PA being an outlier on an optimization question should > never IMHO be a blocker :-) > > jeff
On Thu, Oct 05, 2017 at 10:46:13AM +0200, Richard Biener wrote: > >> PR rtl-optimization/77729 > >> * simplify-rtx.c (simplify_binary_operation_1): Delete the (X&C1)|C2 > >> to (X&(C1&~C2))|C2 transformations. > > OK for the trunk. I'm not sure if the BZ in question qualifies this > > patch for backporting though. Release managers have the final say on > > the backport question. > > We usually avoid doing "optimization regression" backports unless very very > important and obvious. It's very obvious, and quite important, but not very very important so I won't champion backports :-) Segher
diff --git a/gcc/simplify-rtx.c b/gcc/simplify-rtx.c index 1b960b9..3b6cf6f 100644 --- a/gcc/simplify-rtx.c +++ b/gcc/simplify-rtx.c @@ -2673,14 +2673,6 @@ simplify_binary_operation_1 (enum rtx_code code, machine_mode mode, /* If (C1|C2) == ~0 then (X&C1)|C2 becomes X|C2. */ if (((c1|c2) & mask) == mask) return simplify_gen_binary (IOR, mode, XEXP (op0, 0), op1); - - /* Minimize the number of bits set in C1, i.e. C1 := C1 & ~C2. */ - if (((c1 & ~c2) & mask) != (c1 & mask)) - { - tem = simplify_gen_binary (AND, mode, XEXP (op0, 0), - gen_int_mode (c1 & ~c2, mode)); - return simplify_gen_binary (IOR, mode, tem, op1); - } } /* Convert (A & B) | A to A. */ @@ -2736,23 +2728,6 @@ simplify_binary_operation_1 (enum rtx_code code, machine_mode mode, return gen_rtx_ROTATE (int_mode, XEXP (opright, 0), XEXP (SUBREG_REG (opleft), 1)); - /* If we have (ior (and (X C1) C2)), simplify this by making - C1 as small as possible if C1 actually changes. */ - if (CONST_INT_P (op1) - && (HWI_COMPUTABLE_MODE_P (mode) - || INTVAL (op1) > 0) - && GET_CODE (op0) == AND - && CONST_INT_P (XEXP (op0, 1)) - && CONST_INT_P (op1) - && (UINTVAL (XEXP (op0, 1)) & UINTVAL (op1)) != 0) - { - rtx tmp = simplify_gen_binary (AND, mode, XEXP (op0, 0), - gen_int_mode (UINTVAL (XEXP (op0, 1)) - & ~UINTVAL (op1), - mode)); - return simplify_gen_binary (IOR, mode, tmp, op1); - } - /* If OP0 is (ashiftrt (plus ...) C), it might actually be a (sign_extend (plus ...)). Then check if OP1 is a CONST_INT and the PLUS does not affect any of the bits in OP1: then we can do
If we have (X&C1)|C2 simplify_binary_operation_1 makes C1 as small as possible. This makes worse code in common cases like when the AND with C1 is from a zero-extension. This patch fixes it by removing this transformation (twice). I tested this on 31 targets, also some variations that do the transformation only for some conditions (like, do not do it if the resulting constant looks "worse", e.g. has more stretches of ones). 22 of those targets show no difference; 8 are best with this patch variant (never do the transformation); and 64-bit hppa is not best, but the difference is only four insns in a million. Bootstrapped and tested on powerpc64-linux {-m32,-m64}. Is this okay for trunk? And backports? Segher 2017-10-02 Segher Boessenkool <segher@kernel.crashing.org> PR rtl-optimization/77729 * simplify-rtx.c (simplify_binary_operation_1): Delete the (X&C1)|C2 to (X&(C1&~C2))|C2 transformations. --- gcc/simplify-rtx.c | 25 ------------------------- 1 file changed, 25 deletions(-)