[llvm] [MemoryBuiltins] Cache the result of ObjectOffsetSizeVisitor::visit. #64796 (PR #65326)

Bevin Hansson via llvm-commits llvm-commits at lists.llvm.org
Thu Sep 14 23:52:51 PDT 2023


https://github.com/bevin-hansson updated https://github.com/llvm/llvm-project/pull/65326

>From 652b00f27082a64218e41f8a6aebcc7573da2dbc Mon Sep 17 00:00:00 2001
From: Bevin Hansson <bevin.hansson at ericsson.com>
Date: Tue, 5 Sep 2023 14:06:11 +0200
Subject: [PATCH 1/2] [MemoryBuiltins] Cache the result of
 ObjectOffsetSizeVisitor::visit. #64796

visit will skip visiting instructions it already has visited
to avoid issues with cycles in the data graph. However,
the result of this skipping behavior is that if we
encounter the same instruction twice, and that instruction
has a well defined result and isn't part of a cycle, we
will introduce unknowns into the analysis even though we
knew the size and offset of the instruction's result.

Instead of skipping such instructions, keep a cache of
the result of visiting them. This result is initialized
to unknown() before visiting, so if we happen to visit
it again recursively (perhaps as the result of a cycle
or a phi), we will get unknown as the cached result and
exit out.
---
 llvm/include/llvm/Analysis/MemoryBuiltins.h   |  2 +-
 llvm/lib/Analysis/MemoryBuiltins.cpp          | 12 ++--
 .../builtin-object-size-phi.ll                | 56 +++++++++++++++++++
 3 files changed, 65 insertions(+), 5 deletions(-)

diff --git a/llvm/include/llvm/Analysis/MemoryBuiltins.h b/llvm/include/llvm/Analysis/MemoryBuiltins.h
index 711bbf6a0afe5f6..66d1885b92a4905 100644
--- a/llvm/include/llvm/Analysis/MemoryBuiltins.h
+++ b/llvm/include/llvm/Analysis/MemoryBuiltins.h
@@ -198,7 +198,7 @@ class ObjectSizeOffsetVisitor
   ObjectSizeOpts Options;
   unsigned IntTyBits;
   APInt Zero;
-  SmallPtrSet<Instruction *, 8> SeenInsts;
+  DenseMap<Instruction *, SizeOffsetType> SeenInsts;
 
   APInt align(APInt Size, MaybeAlign Align);
 
diff --git a/llvm/lib/Analysis/MemoryBuiltins.cpp b/llvm/lib/Analysis/MemoryBuiltins.cpp
index 53e089ba1feae57..cacebd987f307f1 100644
--- a/llvm/lib/Analysis/MemoryBuiltins.cpp
+++ b/llvm/lib/Analysis/MemoryBuiltins.cpp
@@ -733,10 +733,14 @@ SizeOffsetType ObjectSizeOffsetVisitor::computeImpl(Value *V) {
   if (Instruction *I = dyn_cast<Instruction>(V)) {
     // If we have already seen this instruction, bail out. Cycles can happen in
     // unreachable code after constant propagation.
-    if (!SeenInsts.insert(I).second)
-      return unknown();
-
-    return visit(*I);
+    auto P = SeenInsts.try_emplace(I, unknown());
+    if (!P.second)
+      return P.first->second;
+    SizeOffsetType Res = visit(*I);
+    // Cache the result for later visits. If we happened to visit this during
+    // the above recursion, we would consider it unknown until now.
+    SeenInsts[I] = Res;
+    return Res;
   }
   if (Argument *A = dyn_cast<Argument>(V))
     return visitArgument(*A);
diff --git a/llvm/test/Transforms/LowerConstantIntrinsics/builtin-object-size-phi.ll b/llvm/test/Transforms/LowerConstantIntrinsics/builtin-object-size-phi.ll
index 7937265a69afe1e..60aeef1ff396f71 100644
--- a/llvm/test/Transforms/LowerConstantIntrinsics/builtin-object-size-phi.ll
+++ b/llvm/test/Transforms/LowerConstantIntrinsics/builtin-object-size-phi.ll
@@ -61,3 +61,59 @@ if.end:
   %size = call i64 @llvm.objectsize.i64.p0(ptr %p, i1 true, i1 true, i1 false)
   ret i64 %size
 }
+
+define dso_local i64 @pick_max_same(i32 noundef %n) local_unnamed_addr {
+; CHECK-LABEL: @pick_max_same(
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    [[BUFFER:%.*]] = alloca i8, i64 20, align 1
+; CHECK-NEXT:    [[COND:%.*]] = icmp eq i32 [[N:%.*]], 0
+; CHECK-NEXT:    br i1 [[COND]], label [[IF_ELSE:%.*]], label [[IF_END:%.*]]
+; CHECK:       if.else:
+; CHECK-NEXT:    [[OFFSETED:%.*]] = getelementptr i8, ptr [[BUFFER]], i64 10
+; CHECK-NEXT:    br label [[IF_END]]
+; CHECK:       if.end:
+; CHECK-NEXT:    [[P:%.*]] = phi ptr [ [[OFFSETED]], [[IF_ELSE]] ], [ [[BUFFER]], [[ENTRY:%.*]] ]
+; CHECK-NEXT:    ret i64 20
+;
+entry:
+  %buffer = alloca i8, i64 20
+  %cond = icmp eq i32 %n, 0
+  br i1 %cond, label %if.else, label %if.end
+
+if.else:
+  %offseted = getelementptr i8, ptr %buffer, i64 10
+  br label %if.end
+
+if.end:
+  %p = phi ptr [ %offseted, %if.else ], [ %buffer, %entry ]
+  %size = call i64 @llvm.objectsize.i64.p0(ptr %p, i1 false, i1 true, i1 false)
+  ret i64 %size
+}
+
+define dso_local i64 @pick_min_same(i32 noundef %n) local_unnamed_addr {
+; CHECK-LABEL: @pick_min_same(
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    [[BUFFER:%.*]] = alloca i8, i64 20, align 1
+; CHECK-NEXT:    [[COND:%.*]] = icmp eq i32 [[N:%.*]], 0
+; CHECK-NEXT:    br i1 [[COND]], label [[IF_ELSE:%.*]], label [[IF_END:%.*]]
+; CHECK:       if.else:
+; CHECK-NEXT:    [[OFFSETED:%.*]] = getelementptr i8, ptr [[BUFFER]], i64 10
+; CHECK-NEXT:    br label [[IF_END]]
+; CHECK:       if.end:
+; CHECK-NEXT:    [[P:%.*]] = phi ptr [ [[OFFSETED]], [[IF_ELSE]] ], [ [[BUFFER]], [[ENTRY:%.*]] ]
+; CHECK-NEXT:    ret i64 10
+;
+entry:
+  %buffer = alloca i8, i64 20
+  %cond = icmp eq i32 %n, 0
+  br i1 %cond, label %if.else, label %if.end
+
+if.else:
+  %offseted = getelementptr i8, ptr %buffer, i64 10
+  br label %if.end
+
+if.end:
+  %p = phi ptr [ %offseted, %if.else ], [ %buffer, %entry ]
+  %size = call i64 @llvm.objectsize.i64.p0(ptr %p, i1 true, i1 true, i1 false)
+  ret i64 %size
+}

>From 9cb6e826ad5aa89ef3058321907161f68e76adc8 Mon Sep 17 00:00:00 2001
From: Bevin Hansson <bevin.hansson at ericsson.com>
Date: Fri, 15 Sep 2023 08:51:31 +0200
Subject: [PATCH 2/2] Remove unnecessary attributes.

---
 .../LowerConstantIntrinsics/builtin-object-size-phi.ll        | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/llvm/test/Transforms/LowerConstantIntrinsics/builtin-object-size-phi.ll b/llvm/test/Transforms/LowerConstantIntrinsics/builtin-object-size-phi.ll
index 60aeef1ff396f71..4f4d6a88e1693be 100644
--- a/llvm/test/Transforms/LowerConstantIntrinsics/builtin-object-size-phi.ll
+++ b/llvm/test/Transforms/LowerConstantIntrinsics/builtin-object-size-phi.ll
@@ -62,7 +62,7 @@ if.end:
   ret i64 %size
 }
 
-define dso_local i64 @pick_max_same(i32 noundef %n) local_unnamed_addr {
+define i64 @pick_max_same(i32 %n) {
 ; CHECK-LABEL: @pick_max_same(
 ; CHECK-NEXT:  entry:
 ; CHECK-NEXT:    [[BUFFER:%.*]] = alloca i8, i64 20, align 1
@@ -90,7 +90,7 @@ if.end:
   ret i64 %size
 }
 
-define dso_local i64 @pick_min_same(i32 noundef %n) local_unnamed_addr {
+define i64 @pick_min_same(i32 %n) {
 ; CHECK-LABEL: @pick_min_same(
 ; CHECK-NEXT:  entry:
 ; CHECK-NEXT:    [[BUFFER:%.*]] = alloca i8, i64 20, align 1



More information about the llvm-commits mailing list