[llvm] r218380 - Replace a hand-written suffix compare with std::lexicographical_compare.

[Duncan P. N. Exon Smith]

> On Sep 24, 2014, at 2:57 PM, Benjamin Kramer <benny.kra at gmail.com> wrote:
> 
> 
> 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

I think you can trivially make `<` out of `<=`, since `A < B` is
equivalent to `!(B <= A)`.

I haven't checked your logic, but assuming `A <= B` is:

    [](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()));
    }

then `A < B` should be:

    [](StringRef A, StringRef B) {
        typedef std::reverse_iterator<StringRef::iterator> Reverse;
        return std::lexicographical_compare(Reverse(B.end()), Reverse(B.begin()),
                                            Reverse(A.end()), Reverse(A.begin()));
    }

(Note that I've swapped `A` and `B` and removed the `!`.)