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: Alexander Monakov <amonakov at ispras dot ru>
- To: Maxim Kuvyrkov <maxim dot kuvyrkov at linaro dot org>
- Cc: Wilco Dijkstra <Wilco dot Dijkstra at arm dot com>, Richard Sandiford <richard dot sandiford at linaro dot org>, Christophe Lyon <christophe dot lyon 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: Thu, 5 Oct 2017 23:28:13 +0300 (MSK)
- 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>
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;
}