This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] Fix PR82396: qsort comparator non-negative on sorted output
- From: Christophe Lyon <christophe dot lyon at linaro dot org>
- To: Alexander Monakov <amonakov at ispras dot ru>
- Cc: Maxim Kuvyrkov <maxim dot kuvyrkov at linaro dot org>, Wilco Dijkstra <Wilco dot Dijkstra at arm dot com>, Richard Sandiford <richard dot sandiford at linaro dot org>, GCC Patches <gcc-patches at gcc dot gnu dot org>, nd <nd at arm dot com>, Jim Wilson <wilson at tuliptree dot org>, gnu at the-meissners dot org, Jeff Law <law at redhat dot com>, Vladimir Makarov <vmakarov at redhat dot com>, Steve Ellcey <sellcey at cavium dot com>, Kyrill Tkachov <kyrylo dot tkachov at foss dot arm dot com>
- Date: Fri, 6 Oct 2017 09:55:05 +0200
- Subject: Re: [PATCH] Fix PR82396: qsort comparator non-negative on sorted output
- Authentication-results: sourceware.org; auth=none
- References: <DB6PR0801MB20539F1068D79EF4C510D1AE83730@DB6PR0801MB2053.eurprd08.prod.outlook.com> <1507224043.23982.10.camel@cavium.com> <44B934B2-326E-4937-A5A8-09118ABFA10A@linaro.org> <alpine.LNX.2.20.13.1710052304530.24074@monopod.intra.ispras.ru>
On 5 October 2017 at 22:28, Alexander Monakov <amonakov@ispras.ru> wrote:
> On Thu, 5 Oct 2017, Maxim Kuvyrkov wrote:
>> I'm still working on analysis, but it appears to me that Alexander's patch
>> (current state of trunk) fails qsort check due to not being symmetric for
>> load/store analysis (write == 0 or write == 1) in comparisons with
>> "irrelevant" instructions. Wilco's patch does not seem to address that, and,
>> possibly, makes the failure latent (I may be wrong here, it's late and I
>> didn't finish analysis yet).
>
> Yes, your analysis is incomplete, it should be easy to see that for always-false
> multi_mem_insn_p, autopref_rank_for_schedule implements lexicographical order.
> The problem is that when multi_mem_insn_p may be true, autopref_rank_data is
> not guaranteed to be transitive.
>
> I think your patch loses transitivity in autopref_rank_for_schedule, see Wilco's
> response.
>
> FWIW, this hunk from my patch posted back on Friday is sufficient to restore
> bootstrap as confirmed (again, back on Friday) by Steve. It avoids the fancy
> non-transitive comparison for qsort (but autopref_rank_data is still used in
> multipref_dfa_lookahead_guard).
>
> (I'm surprised Kyrill wasn't Cc'ed - adding him now)
>
> Alexander
>
> * haifa-sched.c (autopref_rank_for_schedule): Do not invoke
> autopref_rank_data, order by min_offset.
>
> diff --git a/gcc/haifa-sched.c b/gcc/haifa-sched.c
> index 549e8961411..cea1242e1f1 100644
> --- a/gcc/haifa-sched.c
> +++ b/gcc/haifa-sched.c
> @@ -5725,7 +5669,7 @@ autopref_rank_for_schedule (const rtx_insn *insn1, const rtx_insn *insn2)
> int irrel2 = data2->status == AUTOPREF_MULTIPASS_DATA_IRRELEVANT;
>
> if (!irrel1 && !irrel2)
> - r = autopref_rank_data (data1, data2);
> + r = data1->min_offset - data2->min_offset;
> else
> r = irrel2 - irrel1;
> }
Hi,
FWIW, I've ran validations with this small patch, and it fixes:
- the aarch64-linux-gnu build problem
- a few other regressions (ICEs) that had probably already been
reported (not sure I saw all the regression reports after r253295)
See http://people.linaro.org/~christophe.lyon/cross-validation/gcc-test-patches/253456-aarch64-bootstrap.patch/report-build-info.html
Where "REF-BUILDFAILED" for aarch64-linux-gnu means the reference
build failed and the patched build succeeded.
The "REGRESSED" cell for aarch64-none-elf with ilp32 only "restores" a
previous failure of gcc.target/aarch64/aapcs64/func-ret-3.c execution,
-O3 -g
Thanks,
Christophe