This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH 4/4] bb-reorder: convert to gcc_stablesort


On Mon, Sep 3, 2018 at 1:17 PM Alexander Monakov <amonakov@ispras.ru> wrote:
>
> On Wed, 29 Aug 2018, Alexander Monakov wrote:
> > On Tue, 28 Aug 2018, Michael Matz wrote:
> > > > I think it's not too complicated, but how about adding this comment:
> > > >
> > > >   profile_count m = c1.max (c2);
> > > >   /* Return 0 if counts are equal, -1 if E1 has the larger count.  */
> > > >   return (m == c2) - (m == c1);
> > > >
> > > > Or, alternatively, employ more branchy checks:
> > > >
> > > >   if (c1 == c2)
> > > >     return 0;
> > > >   if (c1 == c1.max (c2))
> > > >     return -1;
> > > >   return 1;
> > >
> > > You want to at least comment on why a tradition </>/== check isn't
> > > working, namely because of the uninitialized counts.
> >
> > Alright, so how about this comment:
> >
> >   /* Using 'max', as profile_count::operator< does not establish
> >      a strict weak order when uninitialized counts may be present.  */
> >   profile_count m = c1.max (c2);
> >   return (m == c2) - (m == c1);

This variant works for me if you clarify that max () makes uninitialized
counts less.

> > Or the same comment in the "branchy" alternative.
>
> Ping, would any of proposed variants clarify the situation, and were there
> any other issues with the patch?

OK with the above variant.

Richard.

> Thanks.
> Alexander


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]