[llvm] [DominanceFrontier] make iterating dereferenced DominanceFrontierBase::find deterministic (PR #69711)

Wenju He via llvm-commits llvm-commits at lists.llvm.org
Tue Oct 24 18:20:35 PDT 2023


https://github.com/wenju-he updated https://github.com/llvm/llvm-project/pull/69711

>From 3bf2b65dd9ffc2e87b4527a72e82b3728c9be7e3 Mon Sep 17 00:00:00 2001
From: Wenju He <wenju.he at intel.com>
Date: Fri, 20 Oct 2023 19:09:52 +0800
Subject: [PATCH 1/3] [DominanceFrontier] make DominanceFrontierBase::find
 iterator deterministic

When we iterate over a basic block's dominance frontiers returned by
DominanceFrontierBase::find, the order of the dominance frontiers is
indeterministic. This results in indeterministic IR output in our
downstream use case that depends on the order.
---
 llvm/include/llvm/Analysis/DominanceFrontier.h     | 4 ++--
 llvm/include/llvm/Analysis/DominanceFrontierImpl.h | 6 +++---
 llvm/include/llvm/Analysis/RegionInfoImpl.h        | 2 +-
 3 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/llvm/include/llvm/Analysis/DominanceFrontier.h b/llvm/include/llvm/Analysis/DominanceFrontier.h
index cef5e03b3b7a7c7..0f35b45cd2b2b04 100644
--- a/llvm/include/llvm/Analysis/DominanceFrontier.h
+++ b/llvm/include/llvm/Analysis/DominanceFrontier.h
@@ -18,13 +18,13 @@
 #define LLVM_ANALYSIS_DOMINANCEFRONTIER_H
 
 #include "llvm/ADT/GraphTraits.h"
+#include "llvm/ADT/SetVector.h"
 #include "llvm/Config/llvm-config.h"
 #include "llvm/IR/PassManager.h"
 #include "llvm/Pass.h"
 #include "llvm/Support/GenericDomTree.h"
 #include <cassert>
 #include <map>
-#include <set>
 #include <utility>
 
 namespace llvm {
@@ -39,7 +39,7 @@ class raw_ostream;
 template <class BlockT, bool IsPostDom>
 class DominanceFrontierBase {
 public:
-  using DomSetType = std::set<BlockT *>;                // Dom set for a bb
+  using DomSetType = SetVector<BlockT *>;               // Dom set for a bb
   using DomSetMapType = std::map<BlockT *, DomSetType>; // Dom set map
 
 protected:
diff --git a/llvm/include/llvm/Analysis/DominanceFrontierImpl.h b/llvm/include/llvm/Analysis/DominanceFrontierImpl.h
index 3df51d9ad90815c..a9c415f47010943 100644
--- a/llvm/include/llvm/Analysis/DominanceFrontierImpl.h
+++ b/llvm/include/llvm/Analysis/DominanceFrontierImpl.h
@@ -49,7 +49,7 @@ template <class BlockT, bool IsPostDom>
 void DominanceFrontierBase<BlockT, IsPostDom>::removeBlock(BlockT *BB) {
   assert(find(BB) != end() && "Block is not in DominanceFrontier!");
   for (iterator I = begin(), E = end(); I != E; ++I)
-    I->second.erase(BB);
+    I->second.remove(BB);
   Frontiers.erase(BB);
 }
 
@@ -65,7 +65,7 @@ void DominanceFrontierBase<BlockT, IsPostDom>::removeFromFrontier(
     iterator I, BlockT *Node) {
   assert(I != end() && "BB is not in DominanceFrontier!");
   assert(I->second.count(Node) && "Node is not in DominanceFrontier of BB");
-  I->second.erase(Node);
+  I->second.remove(Node);
 }
 
 template <class BlockT, bool IsPostDom>
@@ -133,7 +133,7 @@ void DominanceFrontierBase<BlockT, IsPostDom>::print(raw_ostream &OS) const {
       OS << " <<exit node>>";
     OS << " is:\t";
 
-    const std::set<BlockT *> &BBs = I->second;
+    const SetVector<BlockT *> &BBs = I->second;
 
     for (const BlockT *BB : BBs) {
       OS << ' ';
diff --git a/llvm/include/llvm/Analysis/RegionInfoImpl.h b/llvm/include/llvm/Analysis/RegionInfoImpl.h
index 2877a9f8f0e086a..ec79b35ae324a51 100644
--- a/llvm/include/llvm/Analysis/RegionInfoImpl.h
+++ b/llvm/include/llvm/Analysis/RegionInfoImpl.h
@@ -587,7 +587,7 @@ bool RegionInfoBase<Tr>::isRegion(BlockT *entry, BlockT *exit) const {
   for (BlockT *Succ : *entrySuccs) {
     if (Succ == exit || Succ == entry)
       continue;
-    if (exitSuccs->find(Succ) == exitSuccs->end())
+    if (!exitSuccs->contains(Succ))
       return false;
     if (!isCommonDomFrontier(Succ, entry, exit))
       return false;

>From 4960610c457f5d460957dbc55d43774abe7580a0 Mon Sep 17 00:00:00 2001
From: Wenju He <wenju.he at intel.com>
Date: Wed, 25 Oct 2023 08:14:48 +0800
Subject: [PATCH 2/3] add a comment for SetVector

---
 llvm/include/llvm/Analysis/DominanceFrontier.h | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/llvm/include/llvm/Analysis/DominanceFrontier.h b/llvm/include/llvm/Analysis/DominanceFrontier.h
index 0f35b45cd2b2b04..42ede2ac5ece38d 100644
--- a/llvm/include/llvm/Analysis/DominanceFrontier.h
+++ b/llvm/include/llvm/Analysis/DominanceFrontier.h
@@ -39,7 +39,9 @@ class raw_ostream;
 template <class BlockT, bool IsPostDom>
 class DominanceFrontierBase {
 public:
-  using DomSetType = SetVector<BlockT *>;               // Dom set for a bb
+  // Dom set for a bb. Use SetVector to make iterating dom frontiers of a bb
+  // deterministic.
+  using DomSetType = SetVector<BlockT *>;
   using DomSetMapType = std::map<BlockT *, DomSetType>; // Dom set map
 
 protected:

>From 939bea5e88807165e9b1bd92bb40571c21fd94f4 Mon Sep 17 00:00:00 2001
From: Wenju He <wenju.he at intel.com>
Date: Wed, 25 Oct 2023 09:17:16 +0800
Subject: [PATCH 3/3] check order is deterministic in test

---
 .../Analysis/DominanceFrontier/new_pm_test.ll | 54 +++++++++----------
 1 file changed, 27 insertions(+), 27 deletions(-)

diff --git a/llvm/test/Analysis/DominanceFrontier/new_pm_test.ll b/llvm/test/Analysis/DominanceFrontier/new_pm_test.ll
index c14239c726c76b5..f3dcceacc4d6695 100644
--- a/llvm/test/Analysis/DominanceFrontier/new_pm_test.ll
+++ b/llvm/test/Analysis/DominanceFrontier/new_pm_test.ll
@@ -9,42 +9,42 @@ define void @a_linear_impl_fig_1() nounwind {
 2:
   br label %3
 3:
-  br i1 1, label %12, label %4
+  br i1 1, label %a12, label %4
 4:
   br i1 1, label %5, label %1
 5:
-  br i1 1, label %8, label %6
+  br i1 1, label %a8, label %6
 6:
-  br i1 1, label %7, label %4
-7:
+  br i1 1, label %a7, label %4
+a7:
   ret void
-8:
-  br i1 1, label %9, label %1
-9:
-  br label %10
-10:
-  br i1 1, label %13, label %11
-11:
-  br i1 1, label %9, label %8
-12:
+a8:
+  br i1 1, label %a9, label %1
+a9:
+  br label %a10
+a10:
+  br i1 1, label %a13, label %a11
+a11:
+  br i1 1, label %a9, label %a8
+a12:
   br i1 1, label %2, label %1
-13:
-   switch i32 0, label %1 [ i32 0, label %9
-                              i32 1, label %8]
+a13:
+   switch i32 0, label %1 [ i32 0, label %a9
+                              i32 1, label %a8]
 }
 
 ; CHECK: DominanceFrontier for function: a_linear_impl_fig_1
 ; CHECK-DAG:  DomFrontier for BB %0 is:
-; CHECK-DAG:  DomFrontier for BB %11 is:   %{{[8|9]}} %{{[8|9]}}
+; CHECK-DAG:  DomFrontier for BB %a11 is:   %a9 %a8
 ; CHECK-DAG:  DomFrontier for BB %1 is:    %1
-; CHECK-DAG:  DomFrontier for BB %2 is:    %{{[1|2]}} %{{[1|2]}}
-; CHECK-DAG:  DomFrontier for BB %3 is:    %{{[1|2]}} %{{[1|2]}}
-; CHECK-DAG:  DomFrontier for BB %12 is:   %{{[1|2]}} %{{[1|2]}}
-; CHECK-DAG:  DomFrontier for BB %4 is:    %{{[1|4]}} %{{[1|4]}}
-; CHECK-DAG:  DomFrontier for BB %5 is:    %{{[1|4]}} %{{[1|4]}}
-; CHECK-DAG:  DomFrontier for BB %8 is:    %{{[1|8]}} %{{[1|8]}}
+; CHECK-DAG:  DomFrontier for BB %2 is:    %1 %2
+; CHECK-DAG:  DomFrontier for BB %3 is:    %1 %2
+; CHECK-DAG:  DomFrontier for BB %a12 is:   %2 %1
+; CHECK-DAG:  DomFrontier for BB %4 is:    %1 %4
+; CHECK-DAG:  DomFrontier for BB %5 is:    %4 %1
+; CHECK-DAG:  DomFrontier for BB %a8 is:    %1 %a8
 ; CHECK-DAG:  DomFrontier for BB %6 is:    %4
-; CHECK-DAG:  DomFrontier for BB %7 is:
-; CHECK-DAG:  DomFrontier for BB %9 is:    %{{[1|8|9]}} %{{[1|8|9]}} %{{[1|8|9]}}
-; CHECK-DAG:  DomFrontier for BB %10 is:   %{{[1|8|9]}} %{{[1|8|9]}} %{{[1|8|9]}}
-; CHECK-DAG:  DomFrontier for BB %13 is:   %{{[1|8|9]}} %{{[1|8|9]}} %{{[1|8|9]}}
+; CHECK-DAG:  DomFrontier for BB %a7 is:
+; CHECK-DAG:  DomFrontier for BB %a9 is:    %a9 %a8 %1
+; CHECK-DAG:  DomFrontier for BB %a10 is:   %a9 %a8 %1
+; CHECK-DAG:  DomFrontier for BB %a13 is:   %1 %a9 %a8



More information about the llvm-commits mailing list