This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH 1/5] Fix asymmetric comparison functions
- From: Jakub Jelinek <jakub at redhat dot com>
- To: Yury Gribov <y dot gribov at samsung dot com>
- Cc: Richard Biener <rguenther at suse dot de>, GCC Patches <gcc-patches at gcc dot gnu dot org>, Andrey Belevantsev <abel at ispras dot ru>, Andrew MacLeod <amacleod at redhat dot com>, Andrew Pinski <pinskia at gmail dot com>, Diego Novillo <dnovillo at google dot com>, Geoff Keating <geoffk at geoffk dot org>, Jason Merrill <jason at redhat dot com>, Steven Bosscher <steven at gcc dot gnu dot org>
- Date: Thu, 17 Dec 2015 12:57:20 +0100
- Subject: Re: [PATCH 1/5] Fix asymmetric comparison functions
- Authentication-results: sourceware.org; auth=none
- References: <5672787D dot 6040105 at samsung dot com> <56727936 dot 4030605 at samsung dot com> <alpine dot LSU dot 2 dot 11 dot 1512171239260 dot 3571 at t29 dot fhfr dot qr> <5672A0D1 dot 2040207 at samsung dot com>
- Reply-to: Jakub Jelinek <jakub at redhat dot com>
On Thu, Dec 17, 2015 at 02:47:29PM +0300, Yury Gribov wrote:
> On 12/17/2015 02:41 PM, Richard Biener wrote:
> >On Thu, 17 Dec 2015, Yury Gribov wrote:
> >
> >>Some obvious symmetry fixes.
> >>
> >>Cc-ing
> >>* Andrey (Belevantsev) for bb_top_order_comparator
> >>* Andrew (MacLeod) for compare_case_labels
> >>* Andrew (Pinski) for resort_field_decl_cmp
> >>* Diego for pair_cmp
> >>* Geoff for resort_method_name_cmp
> >>* Jakub for compare_case_labels
> >>* Jason for method_name_cmp
> >>* Richard for insert_phi_nodes_compare_var_infos, compare_case_labels
> >>* Steven for cmp_v_in_regset_pool
> >
> >So for compare_case_labels we only ever have one label with
> >!CASE_LOW - which means you only run into the case that needs
> >!CASE_LOW && !CASE_LOW if comparing an element with itself, correct?
> >
> >In this case (missing "same element" handling rather than symmetry
> >fixing) I'd prefer a
> >
> > if (case1 == case2)
> > return 0;
> >
> >So just to confirm - do the patches also contain same element
> >compare fixings?
>
> Yes, that's a fix for same element. How about adding if + gcc_assert that
> both cases can't be NULL otherwise?
Some of the qsort comparison functions are hot paths, we don't really want
to slow them down. So, if anything, gcc_checking_assert rather than
gcc_assert.
But, which qsort is so lame that it calls comparison on the same element?
Jakub