[llvm] r218380 - Replace a hand-written suffix compare with std::lexicographical_compare.
Benjamin Kramer
benny.kra at gmail.com
Wed Sep 24 14:57:42 PDT 2014
On 24.09.2014, at 23:04, Alexey Samsonov <vonosmas at gmail.com> wrote:
> MSan bootstrap also fails on this:
>
> http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-bootstrap/builds/4564/steps/check-llvm%20msan/logs/stdio
>
> On Wed, Sep 24, 2014 at 1:48 PM, Akira Hatanaka <ahatanak at gmail.com> wrote:
> 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.
The predicate in my code was wrong for std::sort, it's an '<=' instead of a plain '<'. Seems to work
fine with libstdc++ but libc++ falls over :( Introducing A != B would reinstate correctness but has
a potential performance hit in comparison with the original code so I'll leave it as-is for now, was
just cleanup anyways.
- Ben
>
>
> 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
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
>
>
>
> --
> Alexey Samsonov
> vonosmas at gmail.com
More information about the llvm-commits
mailing list