On Tue, Jul 25, 2017 at 10:34 AM, Kyrill Tkachov <kyrylo.tkac...@foss.arm.com> wrote: > Hi, > > On 24/07/17 21:48, Alexander Monakov wrote: >> >> On Sat, 22 Jul 2017, Segher Boessenkool wrote: >> >>> On Sat, Jul 15, 2017 at 11:47:45PM +0300, Alexander Monakov wrote: >>>> >>>> --- a/gcc/gimple-ssa-store-merging.c >>>> +++ b/gcc/gimple-ssa-store-merging.c >>>> @@ -516,12 +516,12 @@ sort_by_bitpos (const void *x, const void *y) >>>> store_immediate_info *const *tmp = (store_immediate_info * const *) >>>> x; >>>> store_immediate_info *const *tmp2 = (store_immediate_info * const *) >>>> y; >>>> - if ((*tmp)->bitpos <= (*tmp2)->bitpos) >>>> + if ((*tmp)->bitpos < (*tmp2)->bitpos) >>>> return -1; >>>> else if ((*tmp)->bitpos > (*tmp2)->bitpos) >>>> return 1; >>>> - >>>> - gcc_unreachable (); >>>> + else >>>> + return 0; >>>> } >>> >>> Does any sort using this comparison function require the sort to be >>> stable? >>> >>> It looks like the <= was simply a typo and should have been <, but >>> the gcc_unreachable was as intended? (With <= it is trivially >>> unreachable there). >> >> I think it's best if the original author can chime in - adding Kyrill. >> >> (to be clear, equal bitpos is possible, this issue causes qsort checker >> errors) > > > For the uses of this function the order when the bitpos is the same > does not matter, I just wanted to avoid returning zero to avoid > perturbations > due to qsort. > IMO the right thing to do here to avoid the qsort checker errors is to break > the tie between > store_immediate_info objects with equal bitpos by using the sort_by_order > heuristic > i.e. something like "return (*tmp)->order - (*tmp2)->order;". > That should give well-behaved results as the order of two > store_immediate_info objects > is guaranteed not to be the same (otherwise something has gone wrong > elsewhere).
Agreed. Richard. > Thanks, > Kyrill > >> Alexander > >
