From: "Richard Earnshaw (lists)" <Richard.Earnshaw@arm.com>
To: Joel Hutton <Joel.Hutton@arm.com>,
Wilco Dijkstra <Wilco.Dijkstra@arm.com>
Cc: nd <nd@arm.com>, GCC Patches <gcc-patches@gcc.gnu.org>
Subject: Re: [AArch64] Use scvtf fbits option where appropriate
Date: Tue, 18 Jun 2019 10:37:00 -0000 [thread overview]
Message-ID: <a2963ff3-11e4-80b2-3985-c8a2b255b723@arm.com> (raw)
In-Reply-To: <3157239f-48e6-bbd6-122c-d173b361bebd@arm.com>
On 18/06/2019 10:11, Joel Hutton wrote:
> Hi,
>
> On 13/06/2019 18:26, Wilco Dijkstra wrote:
>> Wouldn't it be easier to just do exact_log2 (real_to_integer (&r0))
>> and then check the range is in 1..31?
> I've revised this section.
>> --- a/gcc/config/aarch64/aarch64.md
>> +++ b/gcc/config/aarch64/aarch64.md
>> @@ -6016,6 +6016,40 @@
>> [(set_attr "type" "f_cvtf2i")]
>> )
>>
>> +(define_insn "*aarch64_<su_optab>cvtf_<fcvt_target>_<GPF:mode>2_mult"
>> + [(set (match_operand:GPF 0 "register_operand" "=w,w")
>> + (mult:GPF (FLOATUORS:GPF
>> + (match_operand:<FCVT_TARGET> 1 "register_operand" "w,?r"))
>> + (match_operand 2 "aarch64_fp_pow2_recip""Dt,Dt")))]
>>
>> We should add a comment before both define_insn similar to the other
>> conversions, explaining what they do and why there are 2 separate patterns
>> (the default versions of the conversions appear to be missing a comment too).
> I've added comments to the new and existing patterns
>
>
> 0001-SCVTF-fbits.patch
>
> From 5a9dfa6c6eb1c5b9c8c464780b7098058989d472 Mon Sep 17 00:00:00 2001
> From: Joel Hutton <Joel.Hutton@arm.com>
> Date: Thu, 13 Jun 2019 11:08:56 +0100
> Subject: [PATCH] SCVTF fbits
>
> ---
> gcc/config/aarch64/aarch64-protos.h | 1 +
> gcc/config/aarch64/aarch64.c | 28 ++++
> gcc/config/aarch64/aarch64.md | 39 +++++
> gcc/config/aarch64/constraints.md | 7 +
> gcc/config/aarch64/predicates.md | 4 +
> gcc/testsuite/gcc.target/aarch64/fmul_scvtf.c | 140 ++++++++++++++++++
> 6 files changed, 219 insertions(+)
> create mode 100644 gcc/testsuite/gcc.target/aarch64/fmul_scvtf.c
>
> diff --git a/gcc/config/aarch64/aarch64-protos.h b/gcc/config/aarch64/aarch64-protos.h
> index 1e3b1c91db1..ad1ba458a3f 100644
> --- a/gcc/config/aarch64/aarch64-protos.h
> +++ b/gcc/config/aarch64/aarch64-protos.h
> @@ -494,6 +494,7 @@ enum aarch64_symbol_type aarch64_classify_tls_symbol (rtx);
> enum reg_class aarch64_regno_regclass (unsigned);
> int aarch64_asm_preferred_eh_data_format (int, int);
> int aarch64_fpconst_pow_of_2 (rtx);
> +int aarch64_fpconst_pow2_recip (rtx);
> machine_mode aarch64_hard_regno_caller_save_mode (unsigned, unsigned,
> machine_mode);
> int aarch64_uxt_size (int, HOST_WIDE_INT);
> diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
> index 9a035dd9ed8..424ca6c9932 100644
> --- a/gcc/config/aarch64/aarch64.c
> +++ b/gcc/config/aarch64/aarch64.c
> @@ -18707,6 +18707,34 @@ aarch64_fpconst_pow_of_2 (rtx x)
> return exact_log2 (real_to_integer (r));
> }
>
> +/* If X is a positive CONST_DOUBLE with a value that is the reciprocal of a
> + power of 2 (i.e 1/2^n) return the number of float bits. e.g. for x==(1/2^n)
> + return n. Otherwise return -1. */
> +int
> +aarch64_fpconst_pow2_recip (rtx x)
> +{
> + REAL_VALUE_TYPE r0;
> +
> + if (!CONST_DOUBLE_P (x))
> + return -1;
CONST_DOUBLE can be used for things other than floating point. You
should really check that the mode on the double in is in class MODE_FLOAT.
> +
> + r0 = *CONST_DOUBLE_REAL_VALUE (x);
> + if (exact_real_inverse (DFmode, &r0)
> + && !REAL_VALUE_NEGATIVE (r0))
> + {
> + int ret = exact_log2 (real_to_integer (&r0));
> + if (ret >= 1 && ret <= 31)
> + {
> + return ret;
> + }
> + else
> + {
> + return -1;
> + }
> + }
> + return -1;
> +}
> +
> /* If X is a vector of equal CONST_DOUBLE values and that value is
> Y, return the aarch64_fpconst_pow_of_2 of Y. Otherwise return -1. */
>
> diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md
> index 526c7fb0dab..d9812aa238e 100644
> --- a/gcc/config/aarch64/aarch64.md
> +++ b/gcc/config/aarch64/aarch64.md
> @@ -6016,6 +6016,44 @@
> [(set_attr "type" "f_cvtf2i")]
> )
>
> +;; equal width integer to fp combine
> +(define_insn "*aarch64_<su_optab>cvtf_<fcvt_target>_<GPF:mode>2_mult"
> + [(set (match_operand:GPF 0 "register_operand" "=w,w")
> + (mult:GPF (FLOATUORS:GPF
> + (match_operand:<FCVT_TARGET> 1 "register_operand" "w,?r"))
> + (match_operand 2 "aarch64_fp_pow2_recip""Dt,Dt")))]
Missing mode on operand 2. Missing white space between constraint and
predicate.
> + "TARGET_FLOAT"
> + {
> + operands[2] = GEN_INT (aarch64_fpconst_pow2_recip (operands[2]));
> + switch (which_alternative)
> + {
> + case 0:
> + return "<su_optab>cvtf\t%<GPF:s>0, %<s>1, #%2";
> + case 1:
> + return "<su_optab>cvtf\t%<GPF:s>0, %<w1>1, #%2";
> + default:
> + gcc_unreachable();
> + }
> + }
> + [(set_attr "type" "neon_int_to_fp_<Vetype>,f_cvti2f")
> + (set_attr "arch" "simd,fp")]
> +)
> +
> +;; inequal width integer to fp combine
> +(define_insn "*aarch64_<su_optab>cvtf_<fcvt_iesize>_<GPF:mode>2_mult"
> + [(set (match_operand:GPF 0 "register_operand" "=w")
> + (mult:GPF (FLOATUORS:GPF
> + (match_operand:<FCVT_IESIZE> 1 "register_operand" "r"))
> + (match_operand 2 "aarch64_fp_pow2_recip" "Dt")))]
Likewise.
> + "TARGET_FLOAT"
> + {
> + operands[2] = GEN_INT (aarch64_fpconst_pow2_recip (operands[2]));
> + return "<su_optab>cvtf\t%<GPF:s>0, %<w2>1, #%2";
> + }
> + [(set_attr "type" "f_cvti2f")]
> +)
> +
> +;; equal width integer to fp conversion
> (define_insn "<optab><fcvt_target><GPF:mode>2"
> [(set (match_operand:GPF 0 "register_operand" "=w,w")
> (FLOATUORS:GPF (match_operand:<FCVT_TARGET> 1 "register_operand" "w,?r")))]
> @@ -6027,6 +6065,7 @@
> (set_attr "arch" "simd,fp")]
> )
>
> +;; inequal width integer to fp conversions
Start sentences with a capital letter. End them with a full stop.
"inequal" isn't a word: you probably mean "unequal".
> (define_insn "<optab><fcvt_iesize><GPF:mode>2"
> [(set (match_operand:GPF 0 "register_operand" "=w")
> (FLOATUORS:GPF (match_operand:<FCVT_IESIZE> 1 "register_operand" "r")))]
> diff --git a/gcc/config/aarch64/constraints.md b/gcc/config/aarch64/constraints.md
> index 21f9549e660..a7731a033ea 100644
> --- a/gcc/config/aarch64/constraints.md
> +++ b/gcc/config/aarch64/constraints.md
> @@ -329,6 +329,13 @@
> (match_test "aarch64_simd_scalar_immediate_valid_for_move (op,
> QImode)")))
>
> +(define_constraint "Dt"
> + "@internal
> + A const_double which is the reciprocal of an exact power of two, can be
> + used in an scvtf with fract bits operation"
> + (and (match_code "const_double")
> + (match_test "aarch64_fpconst_pow2_recip (op)")))
The test returns -1 on failure, but you're using this as a boolean
predicate (ie != 0).
R.
> +
> (define_constraint "Dl"
> "@internal
> A constraint that matches vector of immediates for left shifts."
> diff --git a/gcc/config/aarch64/predicates.md b/gcc/config/aarch64/predicates.md
> index 10100ca830a..da295981286 100644
> --- a/gcc/config/aarch64/predicates.md
> +++ b/gcc/config/aarch64/predicates.md
> @@ -98,6 +98,10 @@
> (and (match_code "const_double")
> (match_test "aarch64_fpconst_pow_of_2 (op) > 0")))
>
> +(define_predicate "aarch64_fp_pow2_recip"
> + (and (match_code "const_double")
> + (match_test "aarch64_fpconst_pow2_recip (op) > 0")))
> +
> (define_predicate "aarch64_fp_vec_pow2"
> (match_test "aarch64_vec_fpconst_pow_of_2 (op) > 0"))
>
> diff --git a/gcc/testsuite/gcc.target/aarch64/fmul_scvtf.c b/gcc/testsuite/gcc.target/aarch64/fmul_scvtf.c
> new file mode 100644
> index 00000000000..e8d1de6279b
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/aarch64/fmul_scvtf.c
> @@ -0,0 +1,140 @@
> +/* { dg-do run } */
> +/* { dg-options "-save-temps -O2 -fno-inline" } */
> +
> +#define FUNC_DEFS(__a) \
> + float \
> +fsfoo##__a (int x) \
> +{ \
> + return ((float) x)/(1u << __a); \
> +} \
> +float \
> +fusfoo##__a (unsigned int x) \
> +{ \
> + return ((float) x)/(1u << __a); \
> +} \
> +float \
> +fslfoo##__a (long x) \
> +{ \
> + return ((float) x)/(1u << __a); \
> +} \
> +float \
> +fulfoo##__a (unsigned long x) \
> +{ \
> + return ((float) x)/(1u << __a); \
> +} \
> +
> +#define FUNC_DEFD(__a) \
> +double \
> +dsfoo##__a (int x) \
> +{ \
> + return ((double) x)/(1u << __a);\
> +} \
> +double \
> +dusfoo##__a (unsigned int x) \
> +{ \
> + return ((double) x)/(1u << __a);\
> +} \
> +double \
> +dslfoo##__a (long x) \
> +{ \
> + return ((double) x)/(1u << __a);\
> +} \
> +double \
> +dulfoo##__a (unsigned long x) \
> +{ \
> + return ((double) x)/(1u << __a);\
> +}
> +
> +FUNC_DEFS (4)
> + /* { dg-final { scan-assembler-times "scvtf\ts\[0-9\], w\[0-9\]*.*#4" 1 } } */
> + /* { dg-final { scan-assembler-times "ucvtf\ts\[0-9\], w\[0-9\]*.*#4" 1 } } */
> + /* { dg-final { scan-assembler-times "scvtf\ts\[0-9\], x\[0-9\]*.*#4" 1 } } */
> + /* { dg-final { scan-assembler-times "ucvtf\ts\[0-9\], x\[0-9\]*.*#4" 1 } } */
> +
> +FUNC_DEFD (4)
> + /* { dg-final { scan-assembler-times "scvtf\td\[0-9\], w\[0-9\]*.*#4" 1 } } */
> + /* { dg-final { scan-assembler-times "ucvtf\td\[0-9\], w\[0-9\]*.*#4" 1 } } */
> + /* { dg-final { scan-assembler-times "scvtf\td\[0-9\], x\[0-9\]*.*#4" 1 } } */
> + /* { dg-final { scan-assembler-times "ucvtf\td\[0-9\], x\[0-9\]*.*#4" 1 } } */
> +
> +FUNC_DEFS (8)
> + /* { dg-final { scan-assembler-times "scvtf\ts\[0-9\], w\[0-9\]*.*#8" 1 } } */
> + /* { dg-final { scan-assembler-times "ucvtf\ts\[0-9\], w\[0-9\]*.*#8" 1 } } */
> + /* { dg-final { scan-assembler-times "scvtf\ts\[0-9\], x\[0-9\]*.*#8" 1 } } */
> + /* { dg-final { scan-assembler-times "ucvtf\ts\[0-9\], x\[0-9\]*.*#8" 1 } } */
> +
> +FUNC_DEFD (8)
> + /* { dg-final { scan-assembler-times "scvtf\td\[0-9\], w\[0-9\]*.*#8" 1 } } */
> + /* { dg-final { scan-assembler-times "ucvtf\td\[0-9\], w\[0-9\]*.*#8" 1 } } */
> + /* { dg-final { scan-assembler-times "scvtf\td\[0-9\], x\[0-9\]*.*#8" 1 } } */
> + /* { dg-final { scan-assembler-times "ucvtf\td\[0-9\], x\[0-9\]*.*#8" 1 } } */
> +
> +FUNC_DEFS (16)
> + /* { dg-final { scan-assembler-times "scvtf\ts\[0-9\], w\[0-9\]*.*#16" 1 } } */
> + /* { dg-final { scan-assembler-times "ucvtf\ts\[0-9\], w\[0-9\]*.*#16" 1 } } */
> + /* { dg-final { scan-assembler-times "scvtf\ts\[0-9\], x\[0-9\]*.*#16" 1 } } */
> + /* { dg-final { scan-assembler-times "ucvtf\ts\[0-9\], x\[0-9\]*.*#16" 1 } } */
> +
> +FUNC_DEFD (16)
> + /* { dg-final { scan-assembler-times "scvtf\td\[0-9\], w\[0-9\]*.*#16" 1 } } */
> + /* { dg-final { scan-assembler-times "ucvtf\td\[0-9\], w\[0-9\]*.*#16" 1 } } */
> + /* { dg-final { scan-assembler-times "scvtf\td\[0-9\], x\[0-9\]*.*#16" 1 } } */
> + /* { dg-final { scan-assembler-times "ucvtf\td\[0-9\], x\[0-9\]*.*#16" 1 } } */
> +
> +FUNC_DEFS (31)
> + /* { dg-final { scan-assembler-times "scvtf\ts\[0-9\], w\[0-9\]*.*#31" 1 } } */
> + /* { dg-final { scan-assembler-times "ucvtf\ts\[0-9\], w\[0-9\]*.*#31" 1 } } */
> + /* { dg-final { scan-assembler-times "scvtf\ts\[0-9\], x\[0-9\]*.*#31" 1 } } */
> + /* { dg-final { scan-assembler-times "ucvtf\ts\[0-9\], x\[0-9\]*.*#31" 1 } } */
> +
> +FUNC_DEFD (31)
> + /* { dg-final { scan-assembler-times "scvtf\td\[0-9\], w\[0-9\]*.*#31" 1 } } */
> + /* { dg-final { scan-assembler-times "ucvtf\td\[0-9\], w\[0-9\]*.*#31" 1 } } */
> + /* { dg-final { scan-assembler-times "scvtf\td\[0-9\], x\[0-9\]*.*#31" 1 } } */
> + /* { dg-final { scan-assembler-times "ucvtf\td\[0-9\], x\[0-9\]*.*#31" 1 } } */
> +
> +#define FUNC_TESTS(__a, __b) \
> +do \
> +{ \
> + if (fsfoo##__a (__b) != ((int) i) * (1.0f/(1u << __a)) ) \
> + __builtin_abort (); \
> + if (fusfoo##__a (__b) != ((int) i) * (1.0f/(1u << __a)) ) \
> + __builtin_abort (); \
> + if (fslfoo##__a (__b) != ((int) i) * (1.0f/(1u << __a)) ) \
> + __builtin_abort (); \
> + if (fulfoo##__a (__b) != ((int) i) * (1.0f/(1u << __a)) ) \
> + __builtin_abort (); \
> +} while (0)
> +
> +#define FUNC_TESTD(__a, __b) \
> +do \
> +{ \
> + if (fsfoo##__a (__b) != ((int) i) * (1.0d/(1u << __a)) ) \
> + __builtin_abort (); \
> + if (fusfoo##__a (__b) != ((int) i) * (1.0d/(1u << __a)) ) \
> + __builtin_abort (); \
> + if (fslfoo##__a (__b) != ((int) i) * (1.0d/(1u << __a)) ) \
> + __builtin_abort (); \
> + if (fulfoo##__a (__b) != ((int) i) * (1.0d/(1u << __a)) ) \
> + __builtin_abort (); \
> +} while (0)
> +
> + int
> +main (void)
> +{
> + int i;
> +
> + for (i = 0; i < 32; i ++)
> + {
> + FUNC_TESTS (4, i);
> + FUNC_TESTS (8, i);
> + FUNC_TESTS (16, i);
> + FUNC_TESTS (31, i);
> +
> + FUNC_TESTD (4, i);
> + FUNC_TESTD (8, i);
> + FUNC_TESTD (16, i);
> + FUNC_TESTD (31, i);
> + }
> + return 0;
> +}
>
next prev parent reply other threads:[~2019-06-18 10:37 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-06-13 17:26 Wilco Dijkstra
2019-06-18 9:11 ` Joel Hutton
2019-06-18 10:37 ` Richard Earnshaw (lists) [this message]
2019-06-18 11:12 ` Wilco Dijkstra
2019-06-18 12:30 ` Richard Sandiford
2019-06-18 15:34 ` Joel Hutton
2019-06-26 9:35 ` [PING][AArch64] " Joel Hutton
2019-07-01 12:14 ` Wilco Dijkstra
2019-07-01 17:03 ` James Greenhalgh
2019-07-08 16:05 ` Joel Hutton
2019-08-19 17:47 ` James Greenhalgh
-- strict thread matches above, loose matches on Subject: below --
2019-06-13 16:04 [AArch64] " Joel Hutton
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=a2963ff3-11e4-80b2-3985-c8a2b255b723@arm.com \
--to=richard.earnshaw@arm.com \
--cc=Joel.Hutton@arm.com \
--cc=Wilco.Dijkstra@arm.com \
--cc=gcc-patches@gcc.gnu.org \
--cc=nd@arm.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).