[PATCH] D118590: [DFAJumpThreading] make update order deterministic

[Olle Fredriksson via Phabricator via llvm-commits]

ollef created this revision.
ollef added a reviewer: alexey.zhikhar.
Herald added subscribers: mgrang, hiraditya.
ollef requested review of this revision.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

We tracked down some non-determinism in compilation output to the
DFAJumpThreading pass. These changes fixed our issue:

- Make the DefMap type a MapVector to make its iteration order depend on insertion order.
- Sort the values to be inserted into NewDefs by instruction order to make the insertion order deterministic. Since these values come from iterating over a ValueMap, which doesn't have deterministic iteration order, I couldn't fix this at its source.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D118590

Files:
  llvm/lib/Transforms/Scalar/DFAJumpThreading.cpp


Index: llvm/lib/Transforms/Scalar/DFAJumpThreading.cpp
===================================================================
--- llvm/lib/Transforms/Scalar/DFAJumpThreading.cpp
+++ llvm/lib/Transforms/Scalar/DFAJumpThreading.cpp
@@ -357,7 +357,7 @@
 
 // This map keeps track of all the new definitions for an instruction. This
 // information is needed when restoring SSA form after cloning blocks.
-typedef DenseMap<Instruction *, std::vector<Instruction *>> DefMap;
+typedef MapVector<Instruction *, std::vector<Instruction *>> DefMap;
 
 inline raw_ostream &operator<<(raw_ostream &OS, const PathType &Path) {
   OS << "< ";
@@ -1126,6 +1126,9 @@
   /// Add new value mappings to the DefMap to keep track of all new definitions
   /// for a particular instruction. These will be used while updating SSA form.
   void updateDefMap(DefMap &NewDefs, ValueToValueMapTy &VMap) {
+    std::vector<std::pair<Instruction *, Instruction *>> NewDefsVector;
+    NewDefsVector.reserve(VMap.size());
+
     for (auto Entry : VMap) {
       Instruction *Inst =
           dyn_cast<Instruction>(const_cast<Value *>(Entry.first));
@@ -1138,11 +1141,20 @@
       if (!Cloned)
         continue;
 
-      if (NewDefs.find(Inst) == NewDefs.end())
-        NewDefs[Inst] = {Cloned};
-      else
-        NewDefs[Inst].push_back(Cloned);
+      NewDefsVector.push_back({Inst, Cloned});
     }
+
+    // Sort the defs to get deterministic insertion order into NewDefs.
+    sort(NewDefsVector, [](const auto &KV1, const auto &KV2) {
+      auto [Instr1, Cloned1] = KV1;
+      auto [Instr2, Cloned2] = KV2;
+      if (Instr1 == Instr2)
+        return Cloned1->comesBefore(Cloned2);
+      return Instr1->comesBefore(Instr2);
+    });
+
+    for (const auto &[Inst, Cloned] : NewDefsVector)
+      NewDefs[Inst].push_back(Cloned);
   }
 
   /// Update the last branch of a particular cloned path to point to the correct


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D118590.404463.patch
Type: text/x-patch
Size: 1901 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20220131/e86f17e1/attachment.bin>