[PATCH] D39214: [MachineModuleInfoImpls] Replace qsort with std::stable_sort

Benjamin Kramer via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Oct 25 01:20:33 PDT 2017


bkramer requested changes to this revision.
bkramer added a comment.
This revision now requires changes to proceed.

In https://reviews.llvm.org/D39214#906190, @mgrang wrote:

> In https://reviews.llvm.org/D39214#906176, @bkramer wrote:
>
> > Can you point at the test cases that are failing? This would mean that there are two symbols with the same name, which seems like something that shouldn't happen.
>
>
> The following 3 tests fail:
>
>   LLVM :: CodeGen/ARM/available_externally.ll
>   LLVM :: CodeGen/ARM/darwin-tls.ll
>   LLVM :: CodeGen/ARM/indirect-hidden.ll


I think this is just a result of your predicate being wrong for std::sort (it's also wrong for stable_sort, but there you might get lucky). It wants a bool that's true when the symbol is compares less than, instead of the tristate you have in this change.

I'd prefer to just use array_pod_sort here. It also wants a tristate so just swapping out qsort for array_pod_sort should pass tests.


Repository:
  rL LLVM

https://reviews.llvm.org/D39214





More information about the llvm-commits mailing list