[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