Re: [GCC][PATCH v2][Aarch64] Exploiting BFXIL when OR-ing two AND-operations with appropriate bitmasks

[Kyrill Tkachov]

Hi Christophe,

On 18/09/18 23:00, Christophe Lyon wrote:

On Thu, 13 Sep 2018 at 11:49, Kyrill Tkachov
<kyrylo.tkac...@foss.arm.com> wrote:

On 13/09/18 10:25, Sam Tebbs wrote:

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.te...@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.te...@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.

Thanks Sam, I've committed the patch on your behalf with r264264.
If you want to get write-after-approval access to the SVN repo to commit 
patches yourself in the future
please fill out the form at https://sourceware.org/cgi-bin/pdw/ps_form.cgi 
putting my address from the MAINTAINERS file as the approver.

Hi,

You've probably already noticed by now since you fixed the
combine_bfi_1 issue introduced by this commit, but it add another
regression:
FAIL: gcc.target/aarch64/copysign-bsl.c scan-assembler b(sl|it|if)\tv[0-9]

Yeah, that one is a bit more involved as it's an unexpected interaction with 
the copysign BSL pattern.
Would you be able to file a bugzilla issue to track it?

Thanks,
Kyrill

Christophe

Kyrill

Sam