[llvm] r218380 - Replace a hand-written suffix compare with std::lexicographical_compare.
Akira Hatanaka
ahatanak at gmail.com
Wed Sep 24 13:48:36 PDT 2014
Hi Benjamin,
I reverted this commit since it was breaking a few build bots. These two
tests were failing:
Failing Tests (2):
LLVM :: MC/ELF/many-sections-2.s
LLVM :: MC/ELF/many-sections.s
This is the stack dump when many_sections.s was assembled with llvm-mc:
FAIL: LLVM :: MC/ELF/many-sections-2.s (11630 of 11631)
Exit Code: 139
Command Output (stderr):
--
0 llvm-mc 0x000000010994685e
llvm::sys::PrintStackTrace(__sFILE*) + 46
1 llvm-mc 0x0000000109946b6b
PrintStackTraceSignalHandler(void*) + 27
2 llvm-mc 0x0000000109946f95 SignalHandler(int) + 565
3 libsystem_platform.dylib 0x00007fff936bb5aa _sigtramp + 26
4 libsystem_platform.dylib 0x00007fd2c7bef91e _sigtramp + 877872014
5 llvm-mc 0x00000001098e71c0 void
std::__1::__sort<llvm::StringTableBuilder::finalize()::$_0&,
llvm::StringRef*>(llvm::StringRef*, llvm::StringRef*,
llvm::StringTableBuilder::finalize()::$_0&) + 768
6 llvm-mc 0x00000001098e7dad void
std::__1::__sort<llvm::StringTableBuilder::finalize()::$_0&,
llvm::StringRef*>(llvm::StringRef*, llvm::StringRef*,
llvm::StringTableBuilder::finalize()::$_0&) + 3821
7 llvm-mc 0x00000001098e7dad void
std::__1::__sort<llvm::StringTableBuilder::finalize()::$_0&,
llvm::StringRef*>(llvm::StringRef*, llvm::StringRef*,
llvm::StringTableBuilder::finalize()::$_0&) + 3821
8 llvm-mc 0x00000001098e7dad void
std::__1::__sort<llvm::StringTableBuilder::finalize()::$_0&,
llvm::StringRef*>(llvm::StringRef*, llvm::StringRef*,
llvm::StringTableBuilder::finalize()::$_0&) + 3821
9 llvm-mc 0x00000001098e7d69 void
std::__1::__sort<llvm::StringTableBuilder::finalize()::$_0&,
llvm::StringRef*>(llvm::StringRef*, llvm::StringRef*,
llvm::StringTableBuilder::finalize()::$_0&) + 3753
10 llvm-mc 0x00000001098e6c45
llvm::StringTableBuilder::finalize() + 309
11 llvm-mc 0x000000010985081d (anonymous
namespace)::ELFObjectWriter::CreateMetadataSections(llvm::MCAssembler&,
llvm::MCAsmLayout&, llvm::DenseMap<llvm::MCSectionELF const*, unsigned int,
llvm::DenseMapInfo<llvm::MCSectionELF const*> >&,
llvm::DenseMap<llvm::MCSectionELF const*, llvm::MCSectionELF const*,
llvm::DenseMapInfo<llvm::MCSectionELF const*> > const&) + 1293
12 llvm-mc 0x000000010984cf7f (anonymous
namespace)::ELFObjectWriter::WriteObject(llvm::MCAssembler&,
llvm::MCAsmLayout const&) + 47915 llvm-mc
0x00000001098abc05 llvm::MCELFStreamer::FinishImpl() + 69
16 llvm-mc 0x00000001098ce1af llvm::MCStreamer::Finish() +
159
17 llvm-mc 0x00000001097e6cd1 (anonymous
namespace)::AsmParser::Run(bool, bool) + 2689
18 llvm-mc 0x000000010944aacf AssembleInput(char const*,
llvm::Target const*, llvm::SourceMgr&, llvm::MCContext&, llvm::MCStreamer&,
llvm::MCAsmInfo&, llvm::MCSubtargetInfo&, llvm::MCInstrInfo&,
llvm::MCTargetOptions&) + 959
19 llvm-mc 0x0000000109448d79 main + 9977
20 libdyld.dylib 0x00007fff8c0585fd start + 1
Stack dump:
I wonder if the tests are failing because the compare function doesn't
satisfy strict weak ordering. My understanding is that if A == B, comp(A,
B) should return false, but I believe the compare function used in this
patch returns true.
If I check A != B before calling std::lexicographical_compare, the tests
pass.
Let me know if you need more information.
On Wed, Sep 24, 2014 at 6:19 AM, Benjamin Kramer <benny.kra at googlemail.com>
wrote:
> Author: d0k
> Date: Wed Sep 24 08:19:28 2014
> New Revision: 218380
>
> URL: http://llvm.org/viewvc/llvm-project?rev=218380&view=rev
> Log:
> Replace a hand-written suffix compare with std::lexicographical_compare.
>
> No functionality change.
>
> Modified:
> llvm/trunk/lib/MC/StringTableBuilder.cpp
>
> Modified: llvm/trunk/lib/MC/StringTableBuilder.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/MC/StringTableBuilder.cpp?rev=218380&r1=218379&r2=218380&view=diff
>
> ==============================================================================
> --- llvm/trunk/lib/MC/StringTableBuilder.cpp (original)
> +++ llvm/trunk/lib/MC/StringTableBuilder.cpp Wed Sep 24 08:19:28 2014
> @@ -12,25 +12,17 @@
>
> using namespace llvm;
>
> -static bool compareBySuffix(StringRef a, StringRef b) {
> - size_t sizeA = a.size();
> - size_t sizeB = b.size();
> - size_t len = std::min(sizeA, sizeB);
> - for (size_t i = 0; i < len; ++i) {
> - char ca = a[sizeA - i - 1];
> - char cb = b[sizeB - i - 1];
> - if (ca != cb)
> - return ca > cb;
> - }
> - return sizeA > sizeB;
> -}
> -
> void StringTableBuilder::finalize() {
> SmallVector<StringRef, 8> Strings;
> for (auto i = StringIndexMap.begin(), e = StringIndexMap.end(); i != e;
> ++i)
> Strings.push_back(i->getKey());
>
> - std::sort(Strings.begin(), Strings.end(), compareBySuffix);
> + // Sort the vector so a string is sorted above its suffixes.
> + std::sort(Strings.begin(), Strings.end(), [](StringRef A, StringRef B) {
> + typedef std::reverse_iterator<StringRef::iterator> Reverse;
> + return !std::lexicographical_compare(Reverse(A.end()),
> Reverse(A.begin()),
> + Reverse(B.end()),
> Reverse(B.begin()));
> + });
>
> // FIXME: Starting with a null byte is ELF specific. Generalize this so
> we
> // can use the class with other object formats.
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140924/110541b7/attachment.html>
More information about the llvm-commits
mailing list