[llvm-commits] [llvm] r80358 - in /llvm/trunk/lib/Transforms/Instrumentation: MaximumSpanningTree.cpp MaximumSpanningTree.h

[Daniel Dunbar]

On Sun, Aug 30, 2009 at 10:53 AM, Chris Lattner<clattner at apple.com> wrote:
> On Aug 30, 2009, at 1:52 AM, Andreas Neustifter wrote:
>>>
>>> Hi Andreas,
>>>
>>>> +  // compare two weighted edges
>>>> +  struct VISIBILITY_HIDDEN EdgeWeightCompare {
>>>> +    bool operator()(const ProfileInfo::EdgeWeight X,
>>>> +                    const ProfileInfo::EdgeWeight Y) const {
>>>> +      if (X.second > Y.second) return true;
>>>> +      if (X.second < Y.second) return false;
>>>> +#ifndef NDEBUG
>>>
>>> Why does this predicate depend on whether assertions are enabled or
>>> not?  That sounds like a potential source of really insidious bugs.
>>
>> The plan is to have the debug output as stable as possible to be able to
>> compare it to output from previous runs. The weight of the edge
>> (X.second, Y.second) is the only part thats really necessary, the part
>> in the predicated tries to ensure stable sorting of edges of the same
>> weight.
>>
>> But if your experience tells you that this may lead to complications I
>> will remove it.
>
> I don't have a strong opinion, it just looks scary :).  Daniel, what do you
> think?

Since it seems like the extra code is cheap, I would say just leave it
on all the time. Probably makes sense to add a comment that it isn't
strictly necessary, just there to increase determinism.

 - Daniel