From: Michael Collison <Michael.Collison@arm.com>
To: Christophe Lyon <christophe.lyon@linaro.org>,
James Greenhalgh <James.Greenhalgh@arm.com>
Cc: "gcc-patches@gcc.gnu.org" <gcc-patches@gcc.gnu.org>, nd <nd@arm.com>
Subject: RE: [Aarch64][PATCH] Improve Logical And Immediate Expressions
Date: Wed, 23 Nov 2016 16:30:00 -0000 [thread overview]
Message-ID: <HE1PR0802MB237706DB13F8CB4A23C107CD95B70@HE1PR0802MB2377.eurprd08.prod.outlook.com> (raw)
In-Reply-To: <CAKdteObCEovtaf-t7csjXVdRJbbY82dTgFsQcW5pw9r1pWZvYg@mail.gmail.com>
Hi Christophe,
This is not a regression per se; the patch causes the test case to generate one less instruction overall, but one additional 'and'. Trunk before the patch (-O2):
foo:
and w0, w0, 255
lsl w1, w0, 20
orr w0, w1, w0, lsl 8
mov w1, 65024
movk w1, 0xfe0, lsl 16
and w0, w0, w1
ret
The new trunk aarch64 compiler with the patch generates fewer instructions but one additional 'and'
foo:
and w0, w0, 255
lsl w1, w0, 20
orr w0, w1, w0, lsl 8
and x0, x0, 268434944
and x0, x0, -2031617
ret
-----Original Message-----
From: Christophe Lyon [mailto:christophe.lyon@linaro.org]
Sent: Wednesday, November 23, 2016 6:42 AM
To: James Greenhalgh
Cc: Michael Collison; gcc-patches@gcc.gnu.org; nd
Subject: Re: [Aarch64][PATCH] Improve Logical And Immediate Expressions
Hi Michael,
On 21 November 2016 at 10:52, James Greenhalgh <james.greenhalgh@arm.com> wrote:
> On Fri, Nov 18, 2016 at 07:41:58AM +0000, Michael Collison wrote:
>> James,
>>
>> I incorporated all your suggestions, and successfully bootstrapped
>> and re-ran the testsuite.
>>
>> Okay for trunk?
>>
>> 2016-11-18 Michael Collison <michael.collison@arm.com>
>>
>> * config/aarch64/aarch64-protos.h
>> (aarch64_and_split_imm1, aarch64_and_split_imm2)
>> (aarch64_and_bitmask_imm): New prototypes
>> * config/aarch64/aarch64.c (aarch64_and_split_imm1):
>> New overloaded function to create bit mask covering the
>> lowest to highest bits set.
>> (aarch64_and_split_imm2): New overloaded functions to create bit
>> mask of zeros between first and last bit set.
>> (aarch64_and_bitmask_imm): New function to determine if a integer
>> is a valid two instruction "and" operation.
>> * config/aarch64/aarch64.md:(and<mode>3): New define_insn and _split
>> allowing wider range of constants with "and" operations.
>> * (ior<mode>3, xor<mode>3): Use new LOGICAL2 iterator to prevent
>> "and" operator from matching restricted constant range used for
>> ior and xor operators.
>> * config/aarch64/constraints.md (UsO constraint): New SImode constraint
>> for constants in "and" operantions.
>> (UsP constraint): New DImode constraint for constants in "and" operations.
>> * config/aarch64/iterators.md (lconst2): New mode iterator.
>> (LOGICAL2): New code iterator.
>> * config/aarch64/predicates.md (aarch64_logical_and_immediate): New
>> predicate
>> (aarch64_logical_and_operand): New predicate allowing extended constants
>> for "and" operations.
>> * testsuite/gcc.target/aarch64/and_const.c: New test to verify
>> additional constants are recognized and fewer instructions generated.
>> * testsuite/gcc.target/aarch64/and_const2.c: New test to verify
>> additional constants are recognized and fewer instructions generated.
>>
>
>> diff --git a/gcc/config/aarch64/aarch64-protos.h
>> b/gcc/config/aarch64/aarch64-protos.h
>> index 3cdd69b..7ef8cdf 100644
>> --- a/gcc/config/aarch64/aarch64-protos.h
>> +++ b/gcc/config/aarch64/aarch64-protos.h
>> @@ -300,6 +300,9 @@ extern struct tune_params aarch64_tune_params;
>> HOST_WIDE_INT aarch64_initial_elimination_offset (unsigned,
>> unsigned); int aarch64_get_condition_code (rtx); bool
>> aarch64_bitmask_imm (HOST_WIDE_INT val, machine_mode);
>> +unsigned HOST_WIDE_INT aarch64_and_split_imm1 (HOST_WIDE_INT
>> +val_in); unsigned HOST_WIDE_INT aarch64_and_split_imm2
>> +(HOST_WIDE_INT val_in); bool aarch64_and_bitmask_imm (unsigned
>> +HOST_WIDE_INT val_in, machine_mode mode);
>> int aarch64_branch_cost (bool, bool); enum aarch64_symbol_type
>> aarch64_classify_symbolic_expression (rtx); bool
>> aarch64_const_vec_all_same_int_p (rtx, HOST_WIDE_INT); diff --git
>> a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c index
>> 3e663eb..8e33c40 100644
>> --- a/gcc/config/aarch64/aarch64.c
>> +++ b/gcc/config/aarch64/aarch64.c
>> @@ -3600,6 +3600,43 @@ aarch64_bitmask_imm (HOST_WIDE_INT val_in, machine_mode mode)
>> return val == mask * bitmask_imm_mul[__builtin_clz (bits) - 26];
>> }
>>
>> +/* Create mask of ones, covering the lowest to highest bits set in
>> +VAL_IN. */
>> +
>> +unsigned HOST_WIDE_INT
>> +aarch64_and_split_imm1 (HOST_WIDE_INT val_in) {
>> + int lowest_bit_set = ctz_hwi (val_in);
>> + int highest_bit_set = floor_log2 (val_in);
>> + gcc_assert (val_in != 0);
>
> The comment above the function should make this precondition clear. Or
> you could pick a well defined behaviour for when val_in == 0 (return
> 0?), if that fits the rest of the algorithm.
>
> Otherwise, this patch looks OK to me.
>
> Thanks,
> James
>
This patch (r242739) causes a regression on aarch64:
gcc.dg/zero_bits_compound-2.c scan-assembler-times \\(and: 2 now fails.
Christophe.
next prev parent reply other threads:[~2016-11-23 16:30 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-10-27 20:44 Michael Collison
2016-11-17 16:26 ` James Greenhalgh
2016-11-18 7:42 ` Michael Collison
2016-11-21 9:52 ` James Greenhalgh
2016-11-23 13:42 ` Christophe Lyon
2016-11-23 16:30 ` Michael Collison [this message]
2016-11-23 18:43 ` 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=HE1PR0802MB237706DB13F8CB4A23C107CD95B70@HE1PR0802MB2377.eurprd08.prod.outlook.com \
--to=michael.collison@arm.com \
--cc=James.Greenhalgh@arm.com \
--cc=christophe.lyon@linaro.org \
--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).