On Tue, 28 Aug 2018, Richard Biener wrote:
> On Tue, Aug 28, 2018 at 11:22 AM Alexander Monakov <amona...@ispras.ru> wrote:
> >
> > This converts the use in bb-reorder. I had to get a little bit creative
> > with
> > the comparator as profile_count::operator> does not implement a strict weak
> > order.
>
> So the previously used comparator was invalid?
Yes, when one of profile_count values is !initialized_p () (which happens in
testsuite).
> I think your proposed one
> warrants some comments. Maybe trade speed for some clearer code?
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;
> The comments on the operator<> implementations are odd as well:
(an aside, but: it also has strange redundant code like
if (*this == profile_count::zero ())
return false;
if (other == profile_count::zero ())
return !(*this == profile_count::zero ());
where the second return could have been 'return true;')
> /* Comparsions are three-state and conservative. False is returned if
> the inequality can not be decided. */
> bool operator< (const profile_count &other) const
> {
>
> but bool can only represent a single state? Are you supposed to do
>
> bool gt = count1 > count2;
> bool le = count1 <= count2
> if (!gt && !le)
> /* unordered */;
> else if (gt)
> ...
> else if (le)
> ...
>
> ?
*shudder* I assume your question is directed to Honza, but IMO profile_count
comparison APIs could be easier to understand if there was a single function
implementing the meat of comparison and returning one of {unordered, equal,
less, greater}, and remaining operations would simply wrap it.
> And what do we want to do for the unordered case in bb-reorder?
> Tie with current order, thus return zero?
No, that wouldn't be valid for sorting. My patch restores the previous
treatment (uninit/0-frequency edges are worse than any other).
Alexander
