From: Sam Tebbs <sam.tebbs@arm.com>
To: James Greenhalgh <james.greenhalgh@arm.com>,
"gcc-patches@gcc.gnu.org" <gcc-patches@gcc.gnu.org>
Cc: Marcus Shawcroft <Marcus.Shawcroft@arm.com>,
Richard Earnshaw <Richard.Earnshaw@arm.com>, nd <nd@arm.com>
Subject: Re: [GCC][PATCH v2][Aarch64] Exploiting BFXIL when OR-ing two AND-operations with appropriate bitmasks
Date: Thu, 13 Sep 2018 09:49:00 -0000 [thread overview]
Message-ID: <4ab58a71-c586-410c-9fc2-5e9bb0c07de6@arm.com> (raw)
In-Reply-To: <20180911152000.GA37978@arm.com>
[-- Attachment #1: Type: text/plain, Size: 1405 bytes --]
On 09/11/2018 04:20 PM, James Greenhalgh wrote:
> On Tue, Sep 04, 2018 at 10:13:43AM -0500, Sam Tebbs wrote:
>> Hi James,
>>
>> Thanks for the feedback. Here is an update with the changes you proposed
>> and an updated changelog.
>>
>> gcc/
>> 2018-09-04 Sam Tebbs <sam.tebbs@arm.com>
>>
>> PR target/85628
>> * config/aarch64/aarch64.md (*aarch64_bfxil):
>> Define.
>> * config/aarch64/constraints.md (Ulc): Define
>> * config/aarch64/aarch64-protos.h (aarch64_high_bits_all_ones_p):
>> Define.
>> * config/aarch64/aarch64.c (aarch64_high_bits_all_ones_p): New function.
>>
>> gcc/testsuite
>> 2018-09-04 Sam Tebbs <sam.tebbs@arm.com>
>>
>> PR target/85628
>> * gcc.target/aarch64/combine_bfxil.c: New file.
>> * gcc.target/aarch64/combine_bfxil_2.c: New file.
>>
>>
> <snip>
>
>> +/* Return true if I's bits are consecutive ones from the MSB. */
>> +bool
>> +aarch64_high_bits_all_ones_p (HOST_WIDE_INT i)
>> +{
>> + return exact_log2(-i) != HOST_WIDE_INT_M1;
>> +}
> You need a space in here between the function name and the bracket:
>
> exact_log2 (-i)
>
>
>> +extern void abort(void);
> The same comment applies multiple places in this file.
>
> Likewise; if (
>
> Otherwise, OK, please apply with those fixes.
>
> Thanks,
> James
Thanks for noticing that, here's the fixed version.
Sam
[-- Attachment #2: fix.patch --]
[-- Type: text/x-patch, Size: 6848 bytes --]
diff --git a/gcc/config/aarch64/aarch64-protos.h b/gcc/config/aarch64/aarch64-protos.h
index aae1db45ed69c14e306ccce056861a58d9acd834..b26e46f81a414bf71762527f84fd9ac38b81b829 100644
--- a/gcc/config/aarch64/aarch64-protos.h
+++ b/gcc/config/aarch64/aarch64-protos.h
@@ -624,4 +624,6 @@ rtl_opt_pass *make_pass_tag_collision_avoidance (gcc::context *);
poly_uint64 aarch64_regmode_natural_size (machine_mode);
+bool aarch64_high_bits_all_ones_p (HOST_WIDE_INT);
+
#endif /* GCC_AARCH64_PROTOS_H */
diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index d088ef7ee0f256ad0d4f59d2735121de2dd67eba..34acc58510110fbc2cb4abd19ec9d7a04bad3f4c 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -1432,6 +1432,13 @@ aarch64_hard_regno_caller_save_mode (unsigned regno, unsigned,
return SImode;
}
+/* Return true if I's bits are consecutive ones from the MSB. */
+bool
+aarch64_high_bits_all_ones_p (HOST_WIDE_INT i)
+{
+ return exact_log2 (-i) != HOST_WIDE_INT_M1;
+}
+
/* Implement TARGET_CONSTANT_ALIGNMENT. Make strings word-aligned so
that strcpy from constants will be faster. */
diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md
index 955769a64d2030839cdb337321a808626188190e..88f66104db31320389f05cdd5d161db9992a77b8 100644
--- a/gcc/config/aarch64/aarch64.md
+++ b/gcc/config/aarch64/aarch64.md
@@ -5336,6 +5336,31 @@
[(set_attr "type" "rev")]
)
+(define_insn "*aarch64_bfxil<mode>"
+ [(set (match_operand:GPI 0 "register_operand" "=r,r")
+ (ior:GPI (and:GPI (match_operand:GPI 1 "register_operand" "r,0")
+ (match_operand:GPI 3 "const_int_operand" "n, Ulc"))
+ (and:GPI (match_operand:GPI 2 "register_operand" "0,r")
+ (match_operand:GPI 4 "const_int_operand" "Ulc, n"))))]
+ "(INTVAL (operands[3]) == ~INTVAL (operands[4]))
+ && (aarch64_high_bits_all_ones_p (INTVAL (operands[3]))
+ || aarch64_high_bits_all_ones_p (INTVAL (operands[4])))"
+ {
+ switch (which_alternative)
+ {
+ case 0:
+ operands[3] = GEN_INT (ctz_hwi (~INTVAL (operands[3])));
+ return "bfxil\\t%<w>0, %<w>1, 0, %3";
+ case 1:
+ operands[3] = GEN_INT (ctz_hwi (~INTVAL (operands[4])));
+ return "bfxil\\t%<w>0, %<w>2, 0, %3";
+ default:
+ gcc_unreachable ();
+ }
+ }
+ [(set_attr "type" "bfm")]
+)
+
;; There are no canonicalisation rules for the position of the lshiftrt, ashift
;; operations within an IOR/AND RTX, therefore we have two patterns matching
;; each valid permutation.
diff --git a/gcc/config/aarch64/constraints.md b/gcc/config/aarch64/constraints.md
index 72cacdabdac52dcb40b480f7a5bfbf4997c742d8..31fc3eafd8bba03cc773e226223a6293c6dde8d4 100644
--- a/gcc/config/aarch64/constraints.md
+++ b/gcc/config/aarch64/constraints.md
@@ -172,6 +172,13 @@
A constraint that matches the immediate constant -1."
(match_test "op == constm1_rtx"))
+(define_constraint "Ulc"
+ "@internal
+ A constraint that matches a constant integer whose bits are consecutive ones
+ from the MSB."
+ (and (match_code "const_int")
+ (match_test "aarch64_high_bits_all_ones_p (ival)")))
+
(define_constraint "Usv"
"@internal
A constraint that matches a VG-based constant that can be loaded by
diff --git a/gcc/testsuite/gcc.target/aarch64/combine_bfxil.c b/gcc/testsuite/gcc.target/aarch64/combine_bfxil.c
new file mode 100644
index 0000000000000000000000000000000000000000..adb0582ed9d8207f7b52c8912d03345369747448
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/combine_bfxil.c
@@ -0,0 +1,98 @@
+/* { dg-do run } */
+/* { dg-options "-O2 --save-temps" } */
+
+extern void abort (void);
+
+unsigned long long
+combine_balanced (unsigned long long a, unsigned long long b)
+{
+ return (a & 0xffffffff00000000ll) | (b & 0x00000000ffffffffll);
+}
+
+unsigned long long
+combine_minimal (unsigned long long a, unsigned long long b)
+{
+ return (a & 0xfffffffffffffffe) | (b & 0x0000000000000001);
+}
+
+unsigned long long
+combine_unbalanced (unsigned long long a, unsigned long long b)
+{
+ return (a & 0xffffffffff000000ll) | (b & 0x0000000000ffffffll);
+}
+
+unsigned int
+combine_balanced_int (unsigned int a, unsigned int b)
+{
+ return (a & 0xffff0000ll) | (b & 0x0000ffffll);
+}
+
+unsigned int
+combine_unbalanced_int (unsigned int a, unsigned int b)
+{
+ return (a & 0xffffff00ll) | (b & 0x000000ffll);
+}
+
+__attribute__ ((noinline)) void
+foo (unsigned long long a, unsigned long long b, unsigned long long *c,
+ unsigned long long *d)
+{
+ *c = combine_minimal (a, b);
+ *d = combine_minimal (b, a);
+}
+
+__attribute__ ((noinline)) void
+foo2 (unsigned long long a, unsigned long long b, unsigned long long *c,
+ unsigned long long *d)
+{
+ *c = combine_balanced (a, b);
+ *d = combine_balanced (b, a);
+}
+
+__attribute__ ((noinline)) void
+foo3 (unsigned long long a, unsigned long long b, unsigned long long *c,
+ unsigned long long *d)
+{
+ *c = combine_unbalanced (a, b);
+ *d = combine_unbalanced (b, a);
+}
+
+void
+foo4 (unsigned int a, unsigned int b, unsigned int *c, unsigned int *d)
+{
+ *c = combine_balanced_int (a, b);
+ *d = combine_balanced_int (b, a);
+}
+
+void
+foo5 (unsigned int a, unsigned int b, unsigned int *c, unsigned int *d)
+{
+ *c = combine_unbalanced_int (a, b);
+ *d = combine_unbalanced_int (b, a);
+}
+
+int
+main (void)
+{
+ unsigned long long a = 0x0123456789ABCDEF, b = 0xFEDCBA9876543210, c, d;
+ foo3 (a, b, &c, &d);
+ if (c != 0x0123456789543210) abort ();
+ if (d != 0xfedcba9876abcdef) abort ();
+ foo2 (a, b, &c, &d);
+ if (c != 0x0123456776543210) abort ();
+ if (d != 0xfedcba9889abcdef) abort ();
+ foo (a, b, &c, &d);
+ if (c != 0x0123456789abcdee) abort ();
+ if (d != 0xfedcba9876543211) abort ();
+
+ unsigned int a2 = 0x01234567, b2 = 0xFEDCBA98, c2, d2;
+ foo4 (a2, b2, &c2, &d2);
+ if (c2 != 0x0123ba98) abort ();
+ if (d2 != 0xfedc4567) abort ();
+ foo5 (a2, b2, &c2, &d2);
+ if (c2 != 0x01234598) abort ();
+ if (d2 != 0xfedcba67) abort ();
+ return 0;
+}
+
+/* { dg-final { scan-assembler-times "bfxil\\t" 10 } } */
diff --git a/gcc/testsuite/gcc.target/aarch64/combine_bfxil_2.c b/gcc/testsuite/gcc.target/aarch64/combine_bfxil_2.c
new file mode 100644
index 0000000000000000000000000000000000000000..0fc140443bc67bcf12b93d72b7970e095620021e
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/combine_bfxil_2.c
@@ -0,0 +1,16 @@
+/* { dg-do compile } */
+/* { dg-options "-O2" } */
+
+unsigned long long
+combine_non_consecutive (unsigned long long a, unsigned long long b)
+{
+ return (a & 0xfffffff200f00000ll) | (b & 0x00001000ffffffffll);
+}
+
+void
+foo4 (unsigned long long a, unsigned long long b, unsigned long long *c,
+ unsigned long long *d) {
+ /* { dg-final { scan-assembler-not "bfxil\\t" } } */
+ *c = combine_non_consecutive (a, b);
+ *d = combine_non_consecutive (b, a);
+}
next prev parent reply other threads:[~2018-09-13 9:25 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-08-01 15:07 Sam Tebbs
2018-08-28 12:28 ` Sam Tebbs
2018-08-28 22:53 ` James Greenhalgh
2018-08-30 15:54 ` Sam Tebbs
2018-08-31 10:59 ` Kyrill Tkachov
2018-08-31 15:27 ` Sam Tebbs
2018-08-31 15:37 ` Kyrill Tkachov
2018-08-31 15:51 ` Alexander Monakov
2018-08-31 16:39 ` Sam Tebbs
2018-09-04 15:13 ` Sam Tebbs
2018-09-11 15:20 ` James Greenhalgh
2018-09-13 9:49 ` Sam Tebbs [this message]
2018-09-13 9:57 ` Kyrill Tkachov
2018-09-18 22:26 ` Christophe Lyon
2018-09-19 9:42 ` Kyrill Tkachov
2018-09-20 16:37 ` Christophe Lyon
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=4ab58a71-c586-410c-9fc2-5e9bb0c07de6@arm.com \
--to=sam.tebbs@arm.com \
--cc=Marcus.Shawcroft@arm.com \
--cc=Richard.Earnshaw@arm.com \
--cc=gcc-patches@gcc.gnu.org \
--cc=james.greenhalgh@arm.com \
--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).