[llvm] f8d7210 - [GlobalStatus] Keep Visited set in isSafeToDestroyConstant()

[Nikita Popov via llvm-commits]

Author: Nikita Popov
Date: 2022-02-22T10:02:37+01:00
New Revision: f8d72100323b127b39fed5f5890b86d81b8a0335

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

LOG: [GlobalStatus] Keep Visited set in isSafeToDestroyConstant()

Constants cannot be cyclic, but they can be tree-like. Keep a
visited set to ensure we do not degenerate to exponential run-time.

This fixes the problem reported in https://reviews.llvm.org/D117223#3335482,
though I haven't been able to construct a concise test case for
the issue. This requires a combination of dead constants and the
kind of constant expression tree that textual IR cannot represent
(because the textual representation, unlike the in-memory
representation, is also exponential in size).

Added: 
    

Modified: 
    llvm/lib/Transforms/Utils/GlobalStatus.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Transforms/Utils/GlobalStatus.cpp b/llvm/lib/Transforms/Utils/GlobalStatus.cpp
index c1c5f5cc879f5..3ba920cd5878a 100644
--- a/llvm/lib/Transforms/Utils/GlobalStatus.cpp
+++ b/llvm/lib/Transforms/Utils/GlobalStatus.cpp
@@ -38,22 +38,26 @@ static AtomicOrdering strongerOrdering(AtomicOrdering X, AtomicOrdering Y) {
 }
 
 /// It is safe to destroy a constant iff it is only used by constants itself.
-/// Note that constants cannot be cyclic, so this test is pretty easy to
-/// implement recursively.
-///
+/// Note that while constants cannot be cyclic, they can be tree-like, so we
+/// should keep a visited set to avoid exponential runtime.
 bool llvm::isSafeToDestroyConstant(const Constant *C) {
-  if (isa<GlobalValue>(C))
-    return false;
-
-  if (isa<ConstantData>(C))
-    return false;
+  SmallVector<const Constant *, 8> Worklist;
+  SmallPtrSet<const Constant *, 8> Visited;
+  Worklist.push_back(C);
+  while (!Worklist.empty()) {
+    const Constant *C = Worklist.pop_back_val();
+    if (!Visited.insert(C).second)
+      continue;
+    if (isa<GlobalValue>(C) || isa<ConstantData>(C))
+      return false;
 
-  for (const User *U : C->users())
-    if (const Constant *CU = dyn_cast<Constant>(U)) {
-      if (!isSafeToDestroyConstant(CU))
+    for (const User *U : C->users()) {
+      if (const Constant *CU = dyn_cast<Constant>(U))
+        Worklist.push_back(CU);
+      else
         return false;
-    } else
-      return false;
+    }
+  }
   return true;
 }