[PATCH] D143118: [Loads] Return false in canReplacePointersIfEqual helper for non-aliasing pointers

[Usman Nadeem via Phabricator via llvm-commits]

mnadeem created this revision.
mnadeem added reviewers: fhahn, aqjune, efriedma.
Herald added subscribers: jeroen.dobbelaere, StephenFan, hiraditya.
Herald added a project: All.
mnadeem requested review of this revision.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

As suggested in https://discourse.llvm.org/t/gvn-aa-c-should-we-propagate-equalities-for-noalias-pointers/68107


https://reviews.llvm.org/D143118

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
@@ -7,8 +7,12 @@
 //===----------------------------------------------------------------------===//
 
 #include "llvm/Analysis/Loads.h"
+#include "llvm/Analysis/AssumptionCache.h"
+#include "llvm/Analysis/BasicAliasAnalysis.h"
+#include "llvm/Analysis/TargetLibraryInfo.h"
 #include "llvm/AsmParser/Parser.h"
 #include "llvm/IR/Constants.h"
+#include "llvm/IR/Dominators.h"
 #include "llvm/IR/Instructions.h"
 #include "llvm/IR/LLVMContext.h"
 #include "llvm/IR/Module.h"
@@ -71,7 +75,7 @@
 
 declare void @use(i32*)
 
-define void @f(i32* %p) {
+define void @f(i32* %p, i32* noalias %p1) {
   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
@@ -83,16 +87,34 @@
   auto *F = dyn_cast<Function>(GV);
   ASSERT_TRUE(F);
 
+  DominatorTree DT(*F);
+  TargetLibraryInfoImpl TLII;
+  TargetLibraryInfo TLI(TLII);
+  AssumptionCache AC(*F);
+  AAResults AA(TLI);
+  BasicAAResult BAA(DL, *F, TLI, AC, &DT);
+  AA.addAAResult(BAA);
+
   // 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();
+  Value *NoAliasPtr = F->getArg(1);
+
   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));
 
+  // Do not replace pointers if they are known noalias.
+  EXPECT_FALSE(
+      canReplacePointersIfEqual(NoAliasPtr, ConstDerefPtr, DL, nullptr, &AA));
+  EXPECT_FALSE(
+      canReplacePointersIfEqual(ConstDerefPtr, NoAliasPtr, DL, nullptr, &AA));
+  EXPECT_FALSE(canReplacePointersIfEqual(P, NoAliasPtr, DL, nullptr, &AA));
+  EXPECT_FALSE(canReplacePointersIfEqual(NoAliasPtr, P, DL, nullptr, &AA));
+
   ++InstIter;
   Value *ConstUnDerefPtr = *cast<CallInst>(&*InstIter)->arg_begin();
   // ConstUndDerefPtr is a constant pointer that is provably not
Index: llvm/lib/Analysis/Loads.cpp
===================================================================
--- llvm/lib/Analysis/Loads.cpp
+++ llvm/lib/Analysis/Loads.cpp
@@ -685,7 +685,7 @@
 }
 
 bool llvm::canReplacePointersIfEqual(Value *A, Value *B, const DataLayout &DL,
-                                     Instruction *CtxI) {
+                                     Instruction *CtxI, AAResults *AA) {
   Type *Ty = A->getType();
   assert(Ty == B->getType() && Ty->isPointerTy() &&
          "values must have matching pointer types");
@@ -693,6 +693,11 @@
   // 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.
+
+  // Do not replace pointers if they are known noalias.
+  if (AA && AA->alias(A, B) == AliasResult::NoAlias)
+    return false;
+
   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.
Index: llvm/include/llvm/Analysis/Loads.h
===================================================================
--- llvm/include/llvm/Analysis/Loads.h
+++ llvm/include/llvm/Analysis/Loads.h
@@ -180,7 +180,7 @@
 /// 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);
+                               Instruction *CtxI, AAResults *AA = nullptr);
 }
 
 #endif


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D143118.494102.patch
Type: text/x-patch
Size: 4095 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20230202/0edbb532/attachment.bin>