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.
llvm · Feb 4, 2023 · ecb1d84 · ecb1d84
1 parent 76d94d3
commit ecb1d84
Show file tree

Hide file tree

Showing 5 changed files with 13 additions and 16 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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(