[llvm-commits] [llvm] r168074 - /llvm/trunk/lib/CodeGen/StackColoring.cpp

[Benjamin Kramer]

On 16.11.2012, at 14:49, Ulrich Weigand <Ulrich.Weigand at de.ibm.com> wrote:

> Jakob Stoklund Olesen <stoklund at 2pi.dk> wrote on 16.11.2012 00:54:00:
>> On Nov 15, 2012, at 11:33 AM, Ulrich Weigand
>> <Ulrich.Weigand at de.ibm.com> wrote:
>>>  // Sort the slots according to their size. Place unused slots at the
> end.
>>> -  std::sort(SortedSlots.begin(), SortedSlots.end(), SlotSizeSorter
> (MFI));
>>> +  // Use stable sort to guarantee deterministic code generation.
>>> +  std::stable_sort(SortedSlots.begin(), SortedSlots.end(),
>>> +                   SlotSizeSorter(MFI));
>> 
>> Hi Ulrich,
>> 
>> Both stable_sort() and sort() expand into more than 100K of code
>> which is why LLVM provides array_pod_sort() as a wrapper for the
>> libc qsort() function.
>> 
>> In this case, it is very easy to provide a total ordering so you
>> don't need a stable sort.
> 
> Hi Jakob,
> 
> the difficulty I see with using array_pod_sort or something else
> based on qsort is that those just use a plain function as compare
> routine, while std::sort uses a function object ... which is used
> here to hold the "MFI" parameter needed to implement the comparison.
> 
> I don't immediately see how to do this with a plain function
> (except by resorting to global state or nested functions, which
> I'm not sure is better ...).
> 
> Anything I'm overlooking here?  (I'm not a particular C++ expert ...)

Right, array_pod_sort doesn't work here because qsort can't pass the additional MFI parameter.

> I certainly agree that the ordering can easily be made total;
> I could implement this by changing SlotSizeSorter so we can continue
> to use std::sort instead of std::stable_sort.  Would this be
> preferable?

std::sort tends to be slightly faster and expands into less code, though this depends on the stdlib implementation of both algorithms.

- Ben