On Wed, 19 Sep 2018 at 11:31, Kyrill Tkachov
<kyrylo.tkac...@foss.arm.com> wrote:
>
> 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?
>
Sure, this is: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87369
> Thanks,
> Kyrill
>
> > Christophe
> >
> >> Kyrill
> >>
> >>> Sam
>
