On 30/07/2019 11:50, Richard Sandiford wrote:
Richard Biener <richard.guent...@gmail.com> writes:
On Tue, Jul 30, 2019 at 12:04 PM Richard Sandiford
<richard.sandif...@arm.com> wrote:
tree-ssa-math-opts supports FMA optabs for integers as well as
floating-point types, even though there's no distinction between
fused and unfused there. It turns out to be pretty handy for the
IFN_COND_* handling though, so I don't want to remove it, however
weird it might seem.
Instead this patch makes sure that we don't apply it to integer
multiplications that are actually shifts (but that are represented
in gimple as multiplications because that's the canonical form).
This is a preemptive strike. The test doesn't fail for SVE as-is,
but would after a later patch.
Tested on aarch64-linux-gnu, aarch64_be-elf and x86_64-linux-gnu.
OK to install?
Richard
2019-07-30 Richard Sandiford <richard.sandif...@arm.com>
gcc/
* tree-ssa-math-opts.c (convert_mult_to_fma): Reject integer
multiplications by a power of 2 or a negative power of 2.
gcc/testsuite/
* gcc.dg/vect/vect-cond-arith-8.c: New test.
Index: gcc/tree-ssa-math-opts.c
===================================================================
--- gcc/tree-ssa-math-opts.c 2019-07-30 10:51:51.827405171 +0100
+++ gcc/tree-ssa-math-opts.c 2019-07-30 10:52:27.327139149 +0100
@@ -3074,10 +3074,20 @@ convert_mult_to_fma (gimple *mul_stmt, t
&& flag_fp_contract_mode == FP_CONTRACT_OFF)
return false;
- /* We don't want to do bitfield reduction ops. */
- if (INTEGRAL_TYPE_P (type)
- && (!type_has_mode_precision_p (type) || TYPE_OVERFLOW_TRAPS (type)))
- return false;
+ if (ANY_INTEGRAL_TYPE_P (type))
+ {
+ /* We don't want to do bitfield reduction ops. */
+ tree itype = INTEGRAL_TYPE_P (type) ? type : TREE_TYPE (type);
you can use element_type () for this. But I think it's easier to
leave the existing
test in-place since vector or complex types cannot have non-mode precision
components.
Ah, yeah. Guess I was over-generalising here :-)
+ if (!type_has_mode_precision_p (itype) || TYPE_OVERFLOW_TRAPS (itype))
+ return false;
+
+ /* Don't use FMA for multiplications that are actually shifts. */
So I question this - if the FMA can do the shift "for free" and it eventually is
same cost/latency as an add why do we want to not allow this (for all targets)?
Esp. for vector types I would guess a add plus a shift may be more costly
(not all ISAs implement shifts anyhow).
The shift doesn't really come for free. By using FMA we're converting
the shift by a constant into a general multiplication.
Isn't this just the problem that the madd<mode> pattern for aarch64
doesn't accept constants that are a power of 2? So it ends up trying to
force the constant into a register unnecessarily.
(define_insn "madd<mode>"
[(set (match_operand:GPI 0 "register_operand" "=r")
(plus:GPI (mult:GPI (match_operand:GPI 1 "register_operand" "r")
(match_operand:GPI 2 "register_operand" "r"))
--- This should be reg_or_imm_power2, then a second alternative for the
immediate case.
(match_operand:GPI 3 "register_operand" "r")))]
""
R.
Can this be fixed up in the target instead? (by a splitter or appropriate
expander?)
OK, I'll try to do it that way instead.
Thanks,
Richard
+ tree val = VECTOR_TYPE_P (type) ? uniform_vector_p (op2) : op2;
+ if (val
+ && TREE_CODE (val) == INTEGER_CST
+ && wi::popcount (wi::abs (wi::to_wide (val))) == 1)
+ return false;
+ }
/* If the target doesn't support it, don't generate it. We assume that
if fma isn't available then fms, fnma or fnms are not either. */
Index: gcc/testsuite/gcc.dg/vect/vect-cond-arith-8.c
===================================================================
--- /dev/null 2019-07-30 08:53:31.317691683 +0100
+++ gcc/testsuite/gcc.dg/vect/vect-cond-arith-8.c 2019-07-30
10:52:27.327139149 +0100
@@ -0,0 +1,57 @@
+/* { dg-require-effective-target scalar_all_fma } */
+/* { dg-additional-options "-fdump-tree-optimized -ffp-contract=fast" } */
+
+#include "tree-vect.h"
+
+#define N (VECTOR_BITS * 11 / 64 + 3)
+
+#define DEF(INV) \
+ void __attribute__ ((noipa)) \
+ f_##INV (int *restrict a, int *restrict b, \
+ int *restrict c) \
+ { \
+ for (int i = 0; i < N; ++i) \
+ { \
+ int mb = (INV & 1 ? -b[i] : b[i]); \
+ int mc = (INV & 2 ? -c[i] : c[i]); \
+ a[i] = b[i] < 10 ? mb * 8 + mc : 10; \
+ } \
+ }
+
+#define TEST(INV) \
+ { \
+ f_##INV (a, b, c); \
+ for (int i = 0; i < N; ++i) \
+ { \
+ int mb = (INV & 1 ? -b[i] : b[i]); \
+ int mc = (INV & 2 ? -c[i] : c[i]); \
+ int truev = mb * 8 + mc; \
+ if (a[i] != (i % 17 < 10 ? truev : 10)) \
+ __builtin_abort (); \
+ asm volatile ("" ::: "memory"); \
+ } \
+ }
+
+#define FOR_EACH_INV(T) \
+ T (0) T (1) T (2) T (3)
+
+FOR_EACH_INV (DEF)
+
+int
+main (void)
+{
+ int a[N], b[N], c[N];
+ for (int i = 0; i < N; ++i)
+ {
+ b[i] = i % 17;
+ c[i] = i % 13 + 14;
+ asm volatile ("" ::: "memory");
+ }
+ FOR_EACH_INV (TEST)
+ return 0;
+}
+
+/* { dg-final { scan-tree-dump-not { = \.COND_FMA } "optimized" } } */
+/* { dg-final { scan-tree-dump-not { = \.COND_FMS } "optimized" } } */
+/* { dg-final { scan-tree-dump-not { = \.COND_FNMA } "optimized" } } */
+/* { dg-final { scan-tree-dump-not { = \.COND_FNMS } "optimized" } } */
