[Inliner] try more aggressive IR walking to retrieve underlying objec…

[choikwa]

…ts for a given ptr in AddAliasScopeMetadata

getUnderlyingObjectFromPtr will walk over PtrToInt, ptr arithmetic, IntToPtr among many things that llvm::getUnderlyingObject does not. The motivation for this was to allow the inliner to deduce alias scope for memory instructions operating on different addrspaces.

[llvmbot]

@llvm/pr-subscribers-llvm-transforms

Author: choikwa (choikwa)

Changes

…ts for a given ptr in AddAliasScopeMetadata

getUnderlyingObjectFromPtr will walk over PtrToInt, ptr arithmetic, IntToPtr among many things that llvm::getUnderlyingObject does not. The motivation for this was to allow the inliner to deduce alias scope for memory instructions residing in different addrspaces.

---

Full diff: https://github.com/llvm/llvm-project/pull/153122.diff

1 Files Affected:

• (modified) llvm/lib/Transforms/Utils/InlineFunction.cpp (+49-1)

diff --git a/llvm/lib/Transforms/Utils/InlineFunction.cpp b/llvm/lib/Transforms/Utils/InlineFunction.cpp
index fa3c467dd12b9..72a6f4c843494 100644
--- a/llvm/lib/Transforms/Utils/InlineFunction.cpp
+++ b/llvm/lib/Transforms/Utils/InlineFunction.cpp
@@ -1110,6 +1110,54 @@ void ScopedAliasMetadataDeepCloner::remap(Function::iterator FStart,
   }
 }
 
+// Try to retrieve underlying objects from given ptr Value
+static void getUnderlyingObjectsFromPtr(const Value *V,
+                              SmallVectorImpl<const Value *> &Objects,
+                              unsigned int LookupSize = 50) {
+  SmallPtrSet<const Value *, 4> Visited;
+  SmallVector<const Value *, 4> Worklist;
+
+  assert(V->getType()->isPointerTy() && "Must be Ptr type");
+  Worklist.push_back(V);
+  while (!Worklist.empty()) {
+    if (Visited.size() > LookupSize)
+      return;
+
+    const Value *P = Worklist.pop_back_val();
+
+    if (!Visited.insert(P).second) {
+      continue;
+    }
+
+    if (auto *SI = dyn_cast<SelectInst>(P)) {
+      Worklist.push_back(SI->getTrueValue());
+      Worklist.push_back(SI->getFalseValue());
+      continue;
+    }
+
+    if (auto *PN = dyn_cast<PHINode>(P)) {
+      append_range(Worklist, PN->incoming_values());
+      continue;
+    }
+
+    // Handle ptrtoint+arithmetic+inttoptr sequences
+    if (const Instruction *U = dyn_cast<Instruction>(P)) {
+      if (U->getOpcode() == Instruction::IntToPtr ||
+          U->getOpcode() == Instruction::PtrToInt ||
+          U->getOpcode() == Instruction::Add ||
+          U->getOpcode() == Instruction::Mul ||
+          U->getOpcode() == Instruction::Sub ||
+          U->getOpcode() == Instruction::GetElementPtr ||
+          U->isCast()) {
+        append_range(Worklist, U->operands());
+      }
+    } else {
+      // Add leaf nodes
+      Objects.push_back(P);
+    }
+  }
+}
+
 /// If the inlined function has noalias arguments,
 /// then add new alias scopes for each noalias argument, tag the mapped noalias
 /// parameters with noalias metadata specifying the new scope, and tag all
@@ -1247,7 +1295,7 @@ static void AddAliasScopeMetadata(CallBase &CB, ValueToValueMapTy &VMap,
 
       for (const Value *V : PtrArgs) {
         SmallVector<const Value *, 4> Objects;
-        getUnderlyingObjects(V, Objects, /* LI = */ nullptr);
+        getUnderlyingObjectsFromPtr(V, Objects);
 
         ObjSet.insert_range(Objects);
       }

[github-actions]

⚠️ C/C++ code formatter, clang-format found issues in your code. ⚠️

You can test this locally with the following command:

git-clang-format --diff HEAD~1 HEAD --extensions cpp -- llvm/lib/Transforms/Utils/InlineFunction.cpp

View the diff from clang-format here.

diff --git a/llvm/lib/Transforms/Utils/InlineFunction.cpp b/llvm/lib/Transforms/Utils/InlineFunction.cpp
index 72a6f4c84..eeb03fd56 100644
--- a/llvm/lib/Transforms/Utils/InlineFunction.cpp
+++ b/llvm/lib/Transforms/Utils/InlineFunction.cpp
@@ -1112,8 +1112,8 @@ void ScopedAliasMetadataDeepCloner::remap(Function::iterator FStart,
 
 // Try to retrieve underlying objects from given ptr Value
 static void getUnderlyingObjectsFromPtr(const Value *V,
-                              SmallVectorImpl<const Value *> &Objects,
-                              unsigned int LookupSize = 50) {
+                                        SmallVectorImpl<const Value *> &Objects,
+                                        unsigned int LookupSize = 50) {
   SmallPtrSet<const Value *, 4> Visited;
   SmallVector<const Value *, 4> Worklist;
 
@@ -1147,8 +1147,7 @@ static void getUnderlyingObjectsFromPtr(const Value *V,
           U->getOpcode() == Instruction::Add ||
           U->getOpcode() == Instruction::Mul ||
           U->getOpcode() == Instruction::Sub ||
-          U->getOpcode() == Instruction::GetElementPtr ||
-          U->isCast()) {
+          U->getOpcode() == Instruction::GetElementPtr || U->isCast()) {
         append_range(Worklist, U->operands());
       }
     } else {

[choikwa]

Example of IR encountered:
%add.ptr = getelementptr inbounds i16, ptr %some_ptr_arg_noalias, i64 %idx.ext
%0 = ptrtoint ptr %add.ptr to i64
%conv = trunc i64 %0 to i32
%1 = inttoptr i32 %conv to ptr addrspace(3)
%2 = tail call contract <4 x bfloat> @memory_load_call(ptr addrspace(3) %1), !noalias !227

In this case, Inliner calling llvm::getUnderlyingObject would return %1= inttoptr and fail to attach an alias scope to the memory_load_call.

Also, I'm aware of the full restrict patch that's WIP; let me know if that overlaps this.

[ornata]

testcases for each of the underlying object types?

[nikic]

Any provenance based reasoning cannot look through casts between pointers and integers.

If you want to cast between address spaces, use addrspacecast, which preserves provenance.

[choikwa]

Any provenance based reasoning cannot look through casts between pointers and integers.

If you want to cast between address spaces, use addrspacecast, which preserves provenance.

Appreciate the feedback. Looks like the cast from ptr to int came from user code which regrettably did not translate to addrspacecast. Closing and will relay the feedback.