| Message ID | 5624F6E2.6090501@arm.com |
|---|---|
| State | New |
| Headers | show |
Hi Kyrill, On Mon, Oct 19, 2015 at 02:57:54PM +0100, Kyrill Tkachov wrote: > because combine now successfully tries to match: > (set (reg/i:SI 0 x0) > (fix:SI (mult:SF (reg:SF 32 v0 [ a ]) > (const_double:SF 3.2e+1 [0x0.8p+6])))) > > whereas before it would not try the to use the const_double directly > but rather its constant pool reference. What happens if the constant pool reference is actually the better code, do we still generate that? Segher
On 10/19/2015 03:57 PM, Kyrill Tkachov wrote: > This second patch teaches simplify_binary_operation to return the > dereferenced > constants from the constant pool in the binary expression if other > simplifications failed. > > This, combined with the 1/2 patch for aarch64 > (https://gcc.gnu.org/ml/gcc-patches/2015-10/msg01744.html) allow for: > > int > foo (float a) > { > return a * 32.0f; > } > > to generate the code: > foo: > fcvtzs w0, s0, #5 > ret > > because combine now successfully tries to match: > (set (reg/i:SI 0 x0) > (fix:SI (mult:SF (reg:SF 32 v0 [ a ]) > (const_double:SF 3.2e+1 [0x0.8p+6])))) > > whereas before it would not try the to use the const_double directly > but rather its constant pool reference. The only way I could see a problem with that if there are circumstances where the memory variant would simplify further. That doesn't seem highly likely, so... > * simplify-rtx.c (simplify_binary_operation): If either operand was > a constant pool reference use them if all other simplifications > failed. Ok. Bernd
On 19/10/15 15:31, Segher Boessenkool wrote: > Hi Kyrill, Hi Segher, > > On Mon, Oct 19, 2015 at 02:57:54PM +0100, Kyrill Tkachov wrote: >> because combine now successfully tries to match: >> (set (reg/i:SI 0 x0) >> (fix:SI (mult:SF (reg:SF 32 v0 [ a ]) >> (const_double:SF 3.2e+1 [0x0.8p+6])))) >> >> whereas before it would not try the to use the const_double directly >> but rather its constant pool reference. > What happens if the constant pool reference is actually the better > code, do we still generate that? In that case I think the previous calls to simplify_const_binary_operation and simplify_binary_operation_1 should have returned a non-NULL rtx. Kyrill > > > Segher >
Hi Bernd, On 19/10/15 15:31, Bernd Schmidt wrote: > On 10/19/2015 03:57 PM, Kyrill Tkachov wrote: >> This second patch teaches simplify_binary_operation to return the >> dereferenced >> constants from the constant pool in the binary expression if other >> simplifications failed. >> >> This, combined with the 1/2 patch for aarch64 >> (https://gcc.gnu.org/ml/gcc-patches/2015-10/msg01744.html) allow for: >> >> int >> foo (float a) >> { >> return a * 32.0f; >> } >> >> to generate the code: >> foo: >> fcvtzs w0, s0, #5 >> ret >> >> because combine now successfully tries to match: >> (set (reg/i:SI 0 x0) >> (fix:SI (mult:SF (reg:SF 32 v0 [ a ]) >> (const_double:SF 3.2e+1 [0x0.8p+6])))) >> >> whereas before it would not try the to use the const_double directly >> but rather its constant pool reference. > > The only way I could see a problem with that if there are circumstances where the memory variant would simplify further. That doesn't seem highly likely, so... > I that were the case, I'd expect the earlier call to simplify_binary_operation_1 have returned a non-NULL rtx, and the code in this patch would not come into play. >> * simplify-rtx.c (simplify_binary_operation): If either operand was >> a constant pool reference use them if all other simplifications >> failed. > > Ok. Thanks, I'll commit it when the first (aarch64-specific) patch is approved. Kyrill > > > Bernd >
commit f941a03f6ca5dcc0d509490d0e0ec39cefed714b
Author: Kyrylo Tkachov <kyrylo.tkachov@arm.com>
Date: Mon Oct 12 17:12:34 2015 +0100
[simplify-rtx] Use constants from pool when simplifying binops
diff --git a/gcc/simplify-rtx.c b/gcc/simplify-rtx.c
index 5ea5522..519850a 100644
--- a/gcc/simplify-rtx.c
+++ b/gcc/simplify-rtx.c
@@ -2001,7 +2001,17 @@ simplify_binary_operation (enum rtx_code code, machine_mode mode,
tem = simplify_const_binary_operation (code, mode, trueop0, trueop1);
if (tem)
return tem;
- return simplify_binary_operation_1 (code, mode, op0, op1, trueop0, trueop1);
+ tem = simplify_binary_operation_1 (code, mode, op0, op1, trueop0, trueop1);
+
+ if (tem)
+ return tem;
+
+ /* If the above steps did not result in a simplification and op0 or op1
+ were constant pool references, use the referenced constants directly. */
+ if (trueop0 != op0 || trueop1 != op1)
+ return simplify_gen_binary (code, mode, trueop0, trueop1);
+
+ return NULL_RTX;
}
/* Subroutine of simplify_binary_operation. Simplify a binary operation
diff --git a/gcc/testsuite/gcc.target/aarch64/fmul_fcvt_1.c b/gcc/testsuite/gcc.target/aarch64/fmul_fcvt_1.c
index 5af8290..354f2be 100644
--- a/gcc/testsuite/gcc.target/aarch64/fmul_fcvt_1.c
+++ b/gcc/testsuite/gcc.target/aarch64/fmul_fcvt_1.c
@@ -83,6 +83,17 @@ FUNC_DEFD (16)
/* { dg-final { scan-assembler "fcvtzu\tx\[0-9\], d\[0-9\]*.*#4" } } */
/* { dg-final { scan-assembler "fcvtzu\tw\[0-9\], d\[0-9\]*.*#4" } } */
+FUNC_DEFS (32)
+FUNC_DEFD (32)
+/* { dg-final { scan-assembler "fcvtzs\tw\[0-9\], s\[0-9\]*.*#5" } } */
+/* { dg-final { scan-assembler "fcvtzs\tx\[0-9\], s\[0-9\]*.*#5" } } */
+/* { dg-final { scan-assembler "fcvtzs\tx\[0-9\], d\[0-9\]*.*#5" } } */
+/* { dg-final { scan-assembler "fcvtzs\tw\[0-9\], d\[0-9\]*.*#5" } } */
+/* { dg-final { scan-assembler "fcvtzu\tw\[0-9\], s\[0-9\]*.*#5" } } */
+/* { dg-final { scan-assembler "fcvtzu\tx\[0-9\], s\[0-9\]*.*#5" } } */
+/* { dg-final { scan-assembler "fcvtzu\tx\[0-9\], d\[0-9\]*.*#5" } } */
+/* { dg-final { scan-assembler "fcvtzu\tw\[0-9\], d\[0-9\]*.*#5" } } */
+
#define FUNC_TESTS(__a, __b) \
do \
@@ -120,10 +131,12 @@ main (void)
FUNC_TESTS (4, i);
FUNC_TESTS (8, i);
FUNC_TESTS (16, i);
+ FUNC_TESTS (32, i);
FUNC_TESTD (4, i);
FUNC_TESTD (8, i);
FUNC_TESTD (16, i);
+ FUNC_TESTD (32, i);
}
return 0;
}
Hi all, This second patch teaches simplify_binary_operation to return the dereferenced constants from the constant pool in the binary expression if other simplifications failed. This, combined with the 1/2 patch for aarch64 (https://gcc.gnu.org/ml/gcc-patches/2015-10/msg01744.html) allow for: int foo (float a) { return a * 32.0f; } to generate the code: foo: fcvtzs w0, s0, #5 ret because combine now successfully tries to match: (set (reg/i:SI 0 x0) (fix:SI (mult:SF (reg:SF 32 v0 [ a ]) (const_double:SF 3.2e+1 [0x0.8p+6])))) whereas before it would not try the to use the const_double directly but rather its constant pool reference. I've seen this patch trigger once in 453.gromacs from SPEC2006 on aarch64 where it ended up eliminating a floating-point multiplication and a load from a constant pool. There were no other changes, so I reckon this is pretty low impact. Bootstrapped and tested on aarch64, arm, x86_64. CC'ing Eric as this is an RTL optimisation and Segher as this is something that has an effect through combine. Ok for trunk? Thanks, Kyrill 2015-10-19 Kyrylo Tkachov <kyrylo.tkachov@arm.com> * simplify-rtx.c (simplify_binary_operation): If either operand was a constant pool reference use them if all other simplifications failed. 2015-10-19 Kyrylo Tkachov <kyrylo.tkachov@arm.com> * gcc.target/aarch64/fmul_fcvt_1.c: Add multiply-by-32 cases.