On 30/05/18 16:11, Sudakshina Das wrote: > Hi Andre > > On 30/05/18 11:41, Sudakshina Das wrote: >> On 19/05/18 04:12, James Greenhalgh wrote: >>> >>>> On 8 May 2018, at 02:58, Andre Vieira (lists) >>>> <andre.simoesdiasvie...@arm.com> wrote: >>>> >>>> Hi Richard, >>>>> On 07/05/18 11:14, Richard Sandiford wrote: >>>>> "Andre Vieira (lists)" <andre.simoesdiasvie...@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. >> > > I tried with changing the 'false' parameter in your patch to > reload_completed as suggested by Wilco. > > diff --git a/gcc/config/aarch64/predicates.md > b/gcc/config/aarch64/predicates.md > index 4814b93..2d3123d 100644 > --- a/gcc/config/aarch64/predicates.md > +++ b/gcc/config/aarch64/predicates.md > @@ -226,7 +226,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), > false, > + (match_test "aarch64_legitimate_address_p (DFmode, XEXP (op, 0), > reload_completed, > ADDR_QUERY_LDP_STP)"))) > > (define_predicate "aarch64_prefetch_operand" > > But it is still failing on the said benchmark. It is possible that there > is something more hidden that came out because of this patch rather > than the patch itself. > > Would it be possible to temporarily revert this patch for now and look > at it separately? > Yeah I have reverted my patch and am looking into this. I believe the problem arises from a stp with register + immediate offset being printed with offset 8 when it should have been 16.
I'll go try out some changes and should come back with a new patch soon. > Sudi > >> >>> Thanks, >>> James >>> >>> (Trying a mobile mail client, apologies if this bounces) >>> >> >