Re: [Aarch64] PR target/83009: Relax strict address checking for store pair lanes

[Sudakshina Das <sudi dot das at arm dot com>]

On 19/05/18 04:12, James Greenhalgh wrote:
> > On 8 May 2018, at 02:58, Andre Vieira (lists) <Andre.SimoesDiasVieira@arm.com> wrote:
> >
> > Hi Richard,
> > > On 07/05/18 11:14, Richard Sandiford wrote:
> > > "Andre Vieira (lists)" <Andre.SimoesDiasVieira@arm.com> writes:
> > > > Hi,
> > > >
> > > > See below a patch to address PR 83009.
> > > >
> > > > Tested with aarch64-linux-gnu bootstrap and regtests for c, c++ and fortran.
> > > > Ran the adjusted testcase for -mabi=ilp32.
> > > >
> > > > Is this OK for gcc-9?
> > > >
> > > > Cheers,
> > > > Andre
> > > >
> > > > PR target/83009: Relax strict address checking for store pair lanes
> > > >
> > > > The operand constraint for the memory address of store/load pair lanes
> > > > was enforcing strictly hardware registers be allowed as memory
> > > > addresses.  We want to relax that such that these patterns can be used
> > > > by combine.  During register allocation the register constraint will
> > > > enforce the correct register is chosen.
> > >
> > > Nice spot.
> > >
> > > > diff --git a/gcc/config/aarch64/predicates.md b/gcc/config/aarch64/predicates.md
> > > > index 5d41d4350402b2a9e5941f160c6ab6f933bfff90..f29bc8e74f0070589014ac87fd22a95723ba9be8 100644
> > > > --- a/gcc/config/aarch64/predicates.md
> > > > +++ b/gcc/config/aarch64/predicates.md
> > > > @@ -222,7 +222,7 @@
> > > > ;; as a 128-bit vec_concat.
> > > > (define_predicate "aarch64_mem_pair_lanes_operand"
> > > >    (and (match_code "mem")
> > > > -       (match_test "aarch64_legitimate_address_p (DFmode, XEXP (op, 0), 1,
> > > > +       (match_test "aarch64_legitimate_address_p (DFmode, XEXP (op, 0), 0,
> > > >                           ADDR_QUERY_LDP_STP)")))
> > > >
> > > > (define_predicate "aarch64_prefetch_operand"
> > >
> > > Very minor, but it'd be good to change it to a real bool parameter
> > > at the same time, for consistency with aarch64_mem_pair_operand.
> > > (Patch LGTM otherwise FWIW.)
> > Good shout! Thank you. Attached new version.
>
> I’m happy to take Richard’s review here. The patch looks good to me.
>
> Ok for trunk,
> This commit has caused a miscompare failure in Spec2006 434.zeusmp
on aarch64-none-linux-gnu

Running Benchmarks
  Running 434.zeusmp ref base bld default
/home/suddas01/spec2006/src/spec2006/bin/specinvoke -d 
/home/suddas01/spec2006/src/spec2006/benchspec/CPU2006/434.zeusmp/run/run_base_ref_bld.0000 
-e speccmds.err -o speccmds.stdout -f speccmds.cmd -C -q
/home/suddas01/spec2006/src/spec2006/bin/specinvoke -E -d 
/home/suddas01/spec2006/src/spec2006/benchspec/CPU2006/434.zeusmp/run/run_base_ref_bld.0000 
-c 1 -e compare.err -o compare.stdout -f compare.cmd

*** Miscompare of tsl000aa; for details see

/home/suddas01/spec2006/src/spec2006/benchspec/CPU2006/434.zeusmp/run/run_base_ref_bld.0000/tsl000aa.mis
Invalid run; unable to continue.


> Thanks,
> James
>
> (Trying a mobile mail client, apologies if this bounces)