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

[Akira Hatanaka]

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>