[llvm] ecb1d84 - OffloadBinary: Switch to MapVector<StringRef, StringRef> to stabilize iteration order

[Fangrui Song via llvm-commits]

Author: Fangrui Song
Date: 2023-02-04T12:34:55-08:00
New Revision: ecb1d84488431217d7a2902d65d5b326e68bfeb0

URL: https://github.com/llvm/llvm-project/commit/ecb1d84488431217d7a2902d65d5b326e68bfeb0
DIFF: https://github.com/llvm/llvm-project/commit/ecb1d84488431217d7a2902d65d5b326e68bfeb0.diff

LOG: OffloadBinary: Switch to MapVector<StringRef, StringRef> to stabilize iteration order

D122069 incorrectly uses StringMap iteration order
(https://llvm.org/docs/ProgrammersManual.html#llvm-adt-stringmap-h).
Switch to MapVector.

Added: 
    

Modified: 
    llvm/include/llvm/Object/OffloadBinary.h
    llvm/lib/Object/OffloadBinary.cpp
    llvm/lib/ObjectYAML/OffloadEmitter.cpp
    llvm/tools/obj2yaml/offload2yaml.cpp
    llvm/unittests/Object/OffloadingTest.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/include/llvm/Object/OffloadBinary.h b/llvm/include/llvm/Object/OffloadBinary.h
index 72e7e83cfc6bb..320a8e1f6d8ff 100644
--- a/llvm/include/llvm/Object/OffloadBinary.h
+++ b/llvm/include/llvm/Object/OffloadBinary.h
@@ -17,7 +17,7 @@
 #ifndef LLVM_OBJECT_OFFLOADBINARY_H
 #define LLVM_OBJECT_OFFLOADBINARY_H
 
-#include "llvm/ADT/StringMap.h"
+#include "llvm/ADT/MapVector.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Object/Binary.h"
 #include "llvm/Support/Error.h"
@@ -59,7 +59,7 @@ enum ImageKind : uint16_t {
 /// offsets from the beginning of the file.
 class OffloadBinary : public Binary {
 public:
-  using string_iterator = StringMap<StringRef>::const_iterator;
+  using string_iterator = MapVector<StringRef, StringRef>::const_iterator;
   using string_iterator_range = iterator_range<string_iterator>;
 
   /// The current version of the binary used for backwards compatibility.
@@ -70,7 +70,7 @@ class OffloadBinary : public Binary {
     ImageKind TheImageKind;
     OffloadKind TheOffloadKind;
     uint32_t Flags;
-    StringMap<StringRef> StringData;
+    MapVector<StringRef, StringRef> StringData;
     std::unique_ptr<MemoryBuffer> Image;
   };
 
@@ -142,7 +142,7 @@ class OffloadBinary : public Binary {
   OffloadBinary(const OffloadBinary &Other) = delete;
 
   /// Map from keys to offsets in the binary.
-  StringMap<StringRef> StringData;
+  MapVector<StringRef, StringRef> StringData;
   /// Raw pointer to the MemoryBufferRef for convenience.
   const char *Buffer;
   /// Location of the header within the binary.

diff  --git a/llvm/lib/Object/OffloadBinary.cpp b/llvm/lib/Object/OffloadBinary.cpp
index d8cdcdc21d39c..342327daf7e4d 100644
--- a/llvm/lib/Object/OffloadBinary.cpp
+++ b/llvm/lib/Object/OffloadBinary.cpp
@@ -209,8 +209,8 @@ OffloadBinary::write(const OffloadingImage &OffloadingData) {
   // Create a null-terminated string table with all the used strings.
   StringTableBuilder StrTab(StringTableBuilder::ELF);
   for (auto &KeyAndValue : OffloadingData.StringData) {
-    StrTab.add(KeyAndValue.getKey());
-    StrTab.add(KeyAndValue.getValue());
+    StrTab.add(KeyAndValue.first);
+    StrTab.add(KeyAndValue.second);
   }
   StrTab.finalize();
 
@@ -250,8 +250,8 @@ OffloadBinary::write(const OffloadingImage &OffloadingData) {
   OS << StringRef(reinterpret_cast<char *>(&TheEntry), sizeof(Entry));
   for (auto &KeyAndValue : OffloadingData.StringData) {
     uint64_t Offset = sizeof(Header) + sizeof(Entry) + StringEntrySize;
-    StringEntry Map{Offset + StrTab.getOffset(KeyAndValue.getKey()),
-                    Offset + StrTab.getOffset(KeyAndValue.getValue())};
+    StringEntry Map{Offset + StrTab.getOffset(KeyAndValue.first),
+                    Offset + StrTab.getOffset(KeyAndValue.second)};
     OS << StringRef(reinterpret_cast<char *>(&Map), sizeof(StringEntry));
   }
   StrTab.write(OS);

diff  --git a/llvm/lib/ObjectYAML/OffloadEmitter.cpp b/llvm/lib/ObjectYAML/OffloadEmitter.cpp
index 3ffbc4ff0e11e..dfb572531660d 100644
--- a/llvm/lib/ObjectYAML/OffloadEmitter.cpp
+++ b/llvm/lib/ObjectYAML/OffloadEmitter.cpp
@@ -28,12 +28,9 @@ bool yaml2offload(Binary &Doc, raw_ostream &Out, ErrorHandler EH) {
     if (Member.Flags)
       Image.Flags = *Member.Flags;
 
-    StringMap<StringRef> &StringData = Image.StringData;
-    if (Member.StringEntries) {
-      for (const auto &Entry : *Member.StringEntries) {
-        StringData[Entry.Key] = Entry.Value;
-      }
-    }
+    if (Member.StringEntries)
+      for (const auto &Entry : *Member.StringEntries)
+        Image.StringData[Entry.Key] = Entry.Value;
 
     SmallVector<char, 1024> Data;
     raw_svector_ostream OS(Data);

diff  --git a/llvm/tools/obj2yaml/offload2yaml.cpp b/llvm/tools/obj2yaml/offload2yaml.cpp
index 10de0a0fbcb1c..2b63e1278cd22 100644
--- a/llvm/tools/obj2yaml/offload2yaml.cpp
+++ b/llvm/tools/obj2yaml/offload2yaml.cpp
@@ -27,7 +27,7 @@ void populateYAML(OffloadYAML::Binary &YAMLBinary, object::OffloadBinary &OB,
     Member.StringEntries = std::vector<OffloadYAML::Binary::StringEntry>();
     for (const auto &Entry : OB.strings())
       Member.StringEntries->emplace_back(OffloadYAML::Binary::StringEntry(
-          {Saver.save(Entry.getKey()), Saver.save(Entry.getValue())}));
+          {Saver.save(Entry.first), Saver.save(Entry.second)}));
   }
 
   if (!OB.getImage().empty())

diff  --git a/llvm/unittests/Object/OffloadingTest.cpp b/llvm/unittests/Object/OffloadingTest.cpp
index b1be3bd97e1a7..77d8b14a4ffd2 100644
--- a/llvm/unittests/Object/OffloadingTest.cpp
+++ b/llvm/unittests/Object/OffloadingTest.cpp
@@ -30,7 +30,7 @@ TEST(OffloadingTest, checkOffloadingBinary) {
   }
 
   // Create the image.
-  StringMap<StringRef> StringData;
+  MapVector<StringRef, StringRef> StringData;
   for (auto &KeyAndValue : Strings)
     StringData[KeyAndValue.first] = KeyAndValue.second;
   std::unique_ptr<MemoryBuffer> ImageData = MemoryBuffer::getMemBuffer(