Ping.
On Thu, 5 Oct 2017, Alexander Monakov wrote: > In ira-color.c, qsort comparator allocno_priority_compare_func lacks anti- > commutativity and can indicate A < B < A if boths allocnos satisfy > non_spilled_static_chain_regno_p. It should fall down to following > sub-comparisons in that case. > > There is another issue: the comment doesn't match the code. We are sorting > allocnos by priority, higher first, and the comment says that allocnos > corresponding to static chain pointer register should go first. However, > the code implements the opposite ordering. > > I think the comment documents the intended behavior and the code is wrong. > Thus, the patch changes comparison value to match the comment. > > A similar issue was present in lra-assigns.c and was fixed by this patch: > https://gcc.gnu.org/ml/gcc-patches/2017-07/msg00893.html > > Bootstrapped and regtested on x86-64, OK for trunk? > > Thanks. > Alexander > > PR rtl-optimization/82395 > * ira-color.c (allocno_priority_compare_func): Fix comparison step > based on non_spilled_static_chain_regno_p. > > diff --git a/gcc/ira-color.c b/gcc/ira-color.c > index 22fdb88274d..31a4a8074d1 100644 > --- a/gcc/ira-color.c > +++ b/gcc/ira-color.c > @@ -3005,14 +3005,13 @@ allocno_priority_compare_func (const void *v1p, const > void *v2p) > { > ira_allocno_t a1 = *(const ira_allocno_t *) v1p; > ira_allocno_t a2 = *(const ira_allocno_t *) v2p; > - int pri1, pri2; > + int pri1, pri2, diff; > > /* Assign hard reg to static chain pointer pseudo first when > non-local goto is used. */ > - if (non_spilled_static_chain_regno_p (ALLOCNO_REGNO (a1))) > - return 1; > - else if (non_spilled_static_chain_regno_p (ALLOCNO_REGNO (a2))) > - return -1; > + if ((diff = (non_spilled_static_chain_regno_p (ALLOCNO_REGNO (a2)) > + - non_spilled_static_chain_regno_p (ALLOCNO_REGNO (a1)))) != 0) > + return diff; > pri1 = allocno_priorities[ALLOCNO_NUM (a1)]; > pri2 = allocno_priorities[ALLOCNO_NUM (a2)]; > if (pri2 != pri1)
