[PATCH] D85524: [Loads] Add canReplacePointersIfEqual helper.

[Florian Hahn via Phabricator via llvm-commits]

fhahn updated this revision to Diff 287882.
fhahn added a comment.

In D85524#2232554 <https://reviews.llvm.org/D85524#2232554>, @aqjune wrote:

> Hi, I agree this is is a desirable direction.
> Should we have a unit test for this function?

That seems like a good idea, I added a unit test, which also contains a note that the current implementation is incomplete. I also extended the note in the header to state that returning true currently means unknown if they can be replaced.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D85524/new/

https://reviews.llvm.org/D85524

Files:
  llvm/include/llvm/Analysis/Loads.h
  llvm/lib/Analysis/Loads.cpp
  llvm/unittests/Analysis/LoadsTest.cpp


Index: llvm/unittests/Analysis/LoadsTest.cpp
===================================================================
--- llvm/unittests/Analysis/LoadsTest.cpp
+++ llvm/unittests/Analysis/LoadsTest.cpp
@@ -59,3 +59,42 @@
   ASSERT_TRUE(CI);
   ASSERT_TRUE(CI->equalsInt(42));
 }
+
+TEST(LoadsTest, CanReplacePointersIfEqual) {
+  LLVMContext C;
+  std::unique_ptr<Module> M = parseIR(C,
+                                      R"IR(
+ at y = common global [1 x i32] zeroinitializer, align 4
+ at x = common global [1 x i32] zeroinitializer, align 4
+
+declare void @use(i32*)
+
+define void @f(i32* %p) {
+  call void @use(i32* getelementptr inbounds ([1 x i32], [1 x i32]* @y, i64 0, i64 0))
+  call void @use(i32* getelementptr inbounds (i32, i32* getelementptr inbounds ([1 x i32], [1 x i32]* @x, i64 0, i64 0), i64 1))
+  ret void
+}
+)IR");
+  auto &DL = M->getDataLayout();
+  auto *GV = M->getNamedValue("f");
+  ASSERT_TRUE(GV);
+  auto *F = dyn_cast<Function>(GV);
+  ASSERT_TRUE(F);
+
+  // NOTE: the implementation of canReplacePointersIfEqual is incomplete.
+  // Currently the only the cases it returns false for are really sound and
+  // returning true means unknown.
+  Value *P = &*F->arg_begin();
+  auto InstIter = F->front().begin();
+  Value *ConstDerefPtr = *cast<CallInst>(&*InstIter)->arg_begin();
+  // ConstDerefPtr is a constant pointer that is provably de-referenceable. We
+  // can replace an arbitrary pointer with it.
+  EXPECT_TRUE(canReplacePointersIfEqual(P, ConstDerefPtr, DL, nullptr));
+
+  ++InstIter;
+  Value *ConstUnDerefPtr = *cast<CallInst>(&*InstIter)->arg_begin();
+  // ConstUndDerefPtr is a constant pointer that is provably not
+  // de-referenceable. We cannot replace an arbitrary pointer with it.
+  EXPECT_FALSE(
+      canReplacePointersIfEqual(ConstDerefPtr, ConstUnDerefPtr, DL, nullptr));
+}
Index: llvm/lib/Analysis/Loads.cpp
===================================================================
--- llvm/lib/Analysis/Loads.cpp
+++ llvm/lib/Analysis/Loads.cpp
@@ -503,3 +503,23 @@
   // block.
   return nullptr;
 }
+
+bool llvm::canReplacePointersIfEqual(Value *A, Value *B, const DataLayout &DL,
+                                     Instruction *CtxI) {
+  Type *Ty = A->getType();
+  assert(Ty == B->getType() && Ty->isPointerTy() &&
+         "values must have matching pointer types");
+
+  // NOTE: The checks in the function are incomplete and currently miss illegal
+  // cases! The current implementation is a starting point and the
+  // implementation should be made stricter over time.
+  if (auto *C = dyn_cast<Constant>(B)) {
+    // Do not allow replacing a pointer with a constant pointer, unless it is
+    // either null or at least one byte is dereferenceable.
+    APInt OneByte(DL.getPointerTypeSizeInBits(A->getType()), 1);
+    return C->isNullValue() ||
+           isDereferenceableAndAlignedPointer(B, Align(1), OneByte, DL, CtxI);
+  }
+
+  return true;
+}
Index: llvm/include/llvm/Analysis/Loads.h
===================================================================
--- llvm/include/llvm/Analysis/Loads.h
+++ llvm/include/llvm/Analysis/Loads.h
@@ -155,6 +155,15 @@
                                  BasicBlock::iterator &ScanFrom,
                                  unsigned MaxInstsToScan, AAResults *AA,
                                  bool *IsLoadCSE, unsigned *NumScanedInst);
+
+/// Returns true if a pointer value \p A can be replace with another pointer
+/// value \B if they are deemed equal through some means (e.g. information from
+/// conditions).
+/// NOTE: the current implementations is incomplete and unsound. It does not
+/// reject all invalid cases yet, but will be made stricter in the future. In
+/// particular this means returning true means unknown if replacement is safe.
+bool canReplacePointersIfEqual(Value *A, Value *B, const DataLayout &DL,
+                               Instruction *CtxI);
 }
 
 #endif


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D85524.287882.patch
Type: text/x-patch
Size: 3910 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200826/b86f457b/attachment.bin>