[llvm] [ObjectSizeOffsetVisitor] Add a max recursion depth (PR #67479)

Arthur Eubanks via llvm-commits llvm-commits at lists.llvm.org
Tue Sep 26 13:21:21 PDT 2023


https://github.com/aeubanks updated https://github.com/llvm/llvm-project/pull/67479

>From 6e42937cc41cf1d2d5ff3d215253dbf4f6b08fc2 Mon Sep 17 00:00:00 2001
From: Arthur Eubanks <aeubanks at google.com>
Date: Tue, 26 Sep 2023 12:50:08 -0700
Subject: [PATCH 1/3] [ObjectSizeOffsetVisitor] Add a max recursion depth

We're running into stack overflows for huge functions with lots of phis. Even without the stack overflows, this is recursing >7000 in some auto-generated code.

This fixes the stack overflow and brings down the compile time to something reasonable.
---
 llvm/include/llvm/Analysis/MemoryBuiltins.h   |  1 +
 llvm/lib/Analysis/MemoryBuiltins.cpp          | 25 ++++++--
 ...bject-size-offset-visitor-max-recursion.ll | 63 +++++++++++++++++++
 3 files changed, 83 insertions(+), 6 deletions(-)
 create mode 100644 llvm/test/Transforms/DeadStoreElimination/object-size-offset-visitor-max-recursion.ll

diff --git a/llvm/include/llvm/Analysis/MemoryBuiltins.h b/llvm/include/llvm/Analysis/MemoryBuiltins.h
index 1d9831889f573ab..49e5b69898427d1 100644
--- a/llvm/include/llvm/Analysis/MemoryBuiltins.h
+++ b/llvm/include/llvm/Analysis/MemoryBuiltins.h
@@ -199,6 +199,7 @@ class ObjectSizeOffsetVisitor
   unsigned IntTyBits;
   APInt Zero;
   SmallDenseMap<Instruction *, SizeOffsetType, 8> SeenInsts;
+  unsigned RecurseDepth = 0;
 
   APInt align(APInt Size, MaybeAlign Align);
 
diff --git a/llvm/lib/Analysis/MemoryBuiltins.cpp b/llvm/lib/Analysis/MemoryBuiltins.cpp
index 9eab801c3ccbb97..d727e8214dc7938 100644
--- a/llvm/lib/Analysis/MemoryBuiltins.cpp
+++ b/llvm/lib/Analysis/MemoryBuiltins.cpp
@@ -35,6 +35,7 @@
 #include "llvm/IR/Type.h"
 #include "llvm/IR/Value.h"
 #include "llvm/Support/Casting.h"
+#include "llvm/Support/CommandLine.h"
 #include "llvm/Support/Debug.h"
 #include "llvm/Support/MathExtras.h"
 #include "llvm/Support/raw_ostream.h"
@@ -50,6 +51,12 @@ using namespace llvm;
 
 #define DEBUG_TYPE "memory-builtins"
 
+static cl::opt<unsigned> ObjectSizeOffsetVisitorMaxRecurseDepth(
+    "object-size-offset-visitor-max-recurse-depth",
+    cl::desc(
+        "Maximum number of PHIs for ObjectSizeOffsetVisitor to look through"),
+    cl::init(100));
+
 enum AllocType : uint8_t {
   OpNewLike          = 1<<0, // allocates; never returns null
   MallocLike         = 1<<1, // allocates; may return null
@@ -994,14 +1001,20 @@ SizeOffsetType ObjectSizeOffsetVisitor::combineSizeOffset(SizeOffsetType LHS,
 }
 
 SizeOffsetType ObjectSizeOffsetVisitor::visitPHINode(PHINode &PN) {
-  if (PN.getNumIncomingValues() == 0)
+  if (PN.getNumIncomingValues() == 0 ||
+      RecurseDepth >= ObjectSizeOffsetVisitorMaxRecurseDepth)
     return unknown();
+
+  ++RecurseDepth;
   auto IncomingValues = PN.incoming_values();
-  return std::accumulate(IncomingValues.begin() + 1, IncomingValues.end(),
-                         compute(*IncomingValues.begin()),
-                         [this](SizeOffsetType LHS, Value *VRHS) {
-                           return combineSizeOffset(LHS, compute(VRHS));
-                         });
+  SizeOffsetType Ret =
+      std::accumulate(IncomingValues.begin() + 1, IncomingValues.end(),
+                      compute(*IncomingValues.begin()),
+                      [this](SizeOffsetType LHS, Value *VRHS) {
+                        return combineSizeOffset(LHS, compute(VRHS));
+                      });
+  --RecurseDepth;
+  return Ret;
 }
 
 SizeOffsetType ObjectSizeOffsetVisitor::visitSelectInst(SelectInst &I) {
diff --git a/llvm/test/Transforms/DeadStoreElimination/object-size-offset-visitor-max-recursion.ll b/llvm/test/Transforms/DeadStoreElimination/object-size-offset-visitor-max-recursion.ll
new file mode 100644
index 000000000000000..ee6db23f13581d4
--- /dev/null
+++ b/llvm/test/Transforms/DeadStoreElimination/object-size-offset-visitor-max-recursion.ll
@@ -0,0 +1,63 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 3
+; RUN: opt -passes=dse -S -object-size-offset-visitor-max-recurse-depth=1 < %s | FileCheck %s --check-prefix=NO
+; RUN: opt -passes=dse -S -object-size-offset-visitor-max-recurse-depth=2 < %s | FileCheck %s --check-prefix=YES
+
+declare void @use(ptr)
+
+define void @f(i32 %i, i1 %c) {
+; NO-LABEL: define void @f(
+; NO-SAME: i32 [[I:%.*]], i1 [[C:%.*]]) {
+; NO-NEXT:  b0:
+; NO-NEXT:    [[A1:%.*]] = alloca i32, align 4
+; NO-NEXT:    [[A2:%.*]] = alloca i32, align 4
+; NO-NEXT:    br i1 [[C]], label [[B1:%.*]], label [[B2:%.*]]
+; NO:       b1:
+; NO-NEXT:    [[A3:%.*]] = phi ptr [ [[A1]], [[B0:%.*]] ]
+; NO-NEXT:    br label [[B3:%.*]]
+; NO:       b2:
+; NO-NEXT:    [[A4:%.*]] = phi ptr [ [[A2]], [[B0]] ]
+; NO-NEXT:    br label [[B3]]
+; NO:       b3:
+; NO-NEXT:    [[A5:%.*]] = phi ptr [ [[A3]], [[B1]] ], [ [[A4]], [[B2]] ]
+; NO-NEXT:    [[G:%.*]] = getelementptr i8, ptr [[A5]], i32 [[I]]
+; NO-NEXT:    store i8 1, ptr [[G]], align 1
+; NO-NEXT:    store i32 0, ptr [[A5]], align 4
+; NO-NEXT:    call void @use(ptr [[A5]])
+; NO-NEXT:    ret void
+;
+; YES-LABEL: define void @f(
+; YES-SAME: i32 [[I:%.*]], i1 [[C:%.*]]) {
+; YES-NEXT:  b0:
+; YES-NEXT:    [[A1:%.*]] = alloca i32, align 4
+; YES-NEXT:    [[A2:%.*]] = alloca i32, align 4
+; YES-NEXT:    br i1 [[C]], label [[B1:%.*]], label [[B2:%.*]]
+; YES:       b1:
+; YES-NEXT:    [[A3:%.*]] = phi ptr [ [[A1]], [[B0:%.*]] ]
+; YES-NEXT:    br label [[B3:%.*]]
+; YES:       b2:
+; YES-NEXT:    [[A4:%.*]] = phi ptr [ [[A2]], [[B0]] ]
+; YES-NEXT:    br label [[B3]]
+; YES:       b3:
+; YES-NEXT:    [[A5:%.*]] = phi ptr [ [[A3]], [[B1]] ], [ [[A4]], [[B2]] ]
+; YES-NEXT:    store i32 0, ptr [[A5]], align 4
+; YES-NEXT:    call void @use(ptr [[A5]])
+; YES-NEXT:    ret void
+;
+b0:
+  %a1 = alloca i32
+  %a2 = alloca i32
+  br i1 %c, label %b1, label %b2
+b1:
+  %a3 = phi ptr [ %a1, %b0 ]
+  br label %b3
+b2:
+  %a4 = phi ptr [ %a2, %b0 ]
+  br label %b3
+b3:
+  %a5 = phi ptr [ %a3, %b1 ], [ %a4, %b2 ]
+  %g = getelementptr i8, ptr %a5, i32 %i
+  store i8 1, ptr %g
+  store i32 0, ptr %a5
+  call void @use(ptr %a5)
+  ret void
+}

>From 46f553acab2be69f2f991e07cc06a466d3eff5cd Mon Sep 17 00:00:00 2001
From: Arthur Eubanks <aeubanks at google.com>
Date: Tue, 26 Sep 2023 13:16:48 -0700
Subject: [PATCH 2/3] also check selects, lower max depth to 20

---
 llvm/lib/Analysis/MemoryBuiltins.cpp          | 11 ++++--
 ...bject-size-offset-visitor-max-recursion.ll | 37 +++++++++++++++++++
 2 files changed, 45 insertions(+), 3 deletions(-)

diff --git a/llvm/lib/Analysis/MemoryBuiltins.cpp b/llvm/lib/Analysis/MemoryBuiltins.cpp
index d727e8214dc7938..f1f3d3f643fa97c 100644
--- a/llvm/lib/Analysis/MemoryBuiltins.cpp
+++ b/llvm/lib/Analysis/MemoryBuiltins.cpp
@@ -55,7 +55,7 @@ static cl::opt<unsigned> ObjectSizeOffsetVisitorMaxRecurseDepth(
     "object-size-offset-visitor-max-recurse-depth",
     cl::desc(
         "Maximum number of PHIs for ObjectSizeOffsetVisitor to look through"),
-    cl::init(100));
+    cl::init(20));
 
 enum AllocType : uint8_t {
   OpNewLike          = 1<<0, // allocates; never returns null
@@ -1018,8 +1018,13 @@ SizeOffsetType ObjectSizeOffsetVisitor::visitPHINode(PHINode &PN) {
 }
 
 SizeOffsetType ObjectSizeOffsetVisitor::visitSelectInst(SelectInst &I) {
-  return combineSizeOffset(compute(I.getTrueValue()),
-                           compute(I.getFalseValue()));
+  if (RecurseDepth >= ObjectSizeOffsetVisitorMaxRecurseDepth)
+    return unknown();
+  ++RecurseDepth;
+  SizeOffsetType Ret =
+      combineSizeOffset(compute(I.getTrueValue()), compute(I.getFalseValue()));
+  --RecurseDepth;
+  return Ret;
 }
 
 SizeOffsetType ObjectSizeOffsetVisitor::visitUndefValue(UndefValue&) {
diff --git a/llvm/test/Transforms/DeadStoreElimination/object-size-offset-visitor-max-recursion.ll b/llvm/test/Transforms/DeadStoreElimination/object-size-offset-visitor-max-recursion.ll
index ee6db23f13581d4..2339b3803dbbddd 100644
--- a/llvm/test/Transforms/DeadStoreElimination/object-size-offset-visitor-max-recursion.ll
+++ b/llvm/test/Transforms/DeadStoreElimination/object-size-offset-visitor-max-recursion.ll
@@ -61,3 +61,40 @@ b3:
   call void @use(ptr %a5)
   ret void
 }
+
+define void @g(i32 %i, i1 %c1, i1 %c2) {
+; NO-LABEL: define void @g(
+; NO-SAME: i32 [[I:%.*]], i1 [[C1:%.*]], i1 [[C2:%.*]]) {
+; NO-NEXT:    [[A1:%.*]] = alloca i32, align 4
+; NO-NEXT:    [[A2:%.*]] = alloca i32, align 4
+; NO-NEXT:    [[A3:%.*]] = alloca i32, align 4
+; NO-NEXT:    [[A4:%.*]] = select i1 [[C1]], ptr [[A1]], ptr [[A2]]
+; NO-NEXT:    [[A5:%.*]] = select i1 [[C2]], ptr [[A4]], ptr [[A3]]
+; NO-NEXT:    [[G:%.*]] = getelementptr i8, ptr [[A5]], i32 [[I]]
+; NO-NEXT:    store i8 1, ptr [[G]], align 1
+; NO-NEXT:    store i32 0, ptr [[A5]], align 4
+; NO-NEXT:    call void @use(ptr [[A5]])
+; NO-NEXT:    ret void
+;
+; YES-LABEL: define void @g(
+; YES-SAME: i32 [[I:%.*]], i1 [[C1:%.*]], i1 [[C2:%.*]]) {
+; YES-NEXT:    [[A1:%.*]] = alloca i32, align 4
+; YES-NEXT:    [[A2:%.*]] = alloca i32, align 4
+; YES-NEXT:    [[A3:%.*]] = alloca i32, align 4
+; YES-NEXT:    [[A4:%.*]] = select i1 [[C1]], ptr [[A1]], ptr [[A2]]
+; YES-NEXT:    [[A5:%.*]] = select i1 [[C2]], ptr [[A4]], ptr [[A3]]
+; YES-NEXT:    store i32 0, ptr [[A5]], align 4
+; YES-NEXT:    call void @use(ptr [[A5]])
+; YES-NEXT:    ret void
+;
+  %a1 = alloca i32
+  %a2 = alloca i32
+  %a3 = alloca i32
+  %a4 = select i1 %c1, ptr %a1, ptr %a2
+  %a5 = select i1 %c2, ptr %a4, ptr %a3
+  %g = getelementptr i8, ptr %a5, i32 %i
+  store i8 1, ptr %g
+  store i32 0, ptr %a5
+  call void @use(ptr %a5)
+  ret void
+}

>From 8aa386652c0c925c6b3b9d9d17f618eeec749158 Mon Sep 17 00:00:00 2001
From: Arthur Eubanks <aeubanks at google.com>
Date: Tue, 26 Sep 2023 13:20:08 -0700
Subject: [PATCH 3/3] fix cl::opt description

---
 llvm/lib/Analysis/MemoryBuiltins.cpp | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/llvm/lib/Analysis/MemoryBuiltins.cpp b/llvm/lib/Analysis/MemoryBuiltins.cpp
index f1f3d3f643fa97c..51aeb7f0e7fba48 100644
--- a/llvm/lib/Analysis/MemoryBuiltins.cpp
+++ b/llvm/lib/Analysis/MemoryBuiltins.cpp
@@ -53,8 +53,8 @@ using namespace llvm;
 
 static cl::opt<unsigned> ObjectSizeOffsetVisitorMaxRecurseDepth(
     "object-size-offset-visitor-max-recurse-depth",
-    cl::desc(
-        "Maximum number of PHIs for ObjectSizeOffsetVisitor to look through"),
+    cl::desc("Maximum number of PHIs/selects for ObjectSizeOffsetVisitor to "
+             "look through"),
     cl::init(20));
 
 enum AllocType : uint8_t {



More information about the llvm-commits mailing list