Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BasicAA] Remove incorrect rule about constant pointers #76815

Merged
merged 1 commit into from
Jan 17, 2024

Conversation

nikic
Copy link
Contributor

@nikic nikic commented Jan 3, 2024

BasicAA currently says that any Constant cannot alias an identified local object. This is not correct if the local object escaped, as it's possible to create a pointer to the escaped object using an inttoptr constant expression base.

To compensate for this, make sure that inttoptr constant expressions are treated as escape sources, just like inttoptr instructions. This ensures that the optimization can still be applied if the local object is non-escaping. This is sufficient to still optimize the original motivation case from c53e2ec.

Fixes #76789.

BasicAA currently says that any Constant cannot alias an identified
local object. This is not correct if the local object escaped, as
it's possible to create a pointer to the escaped object using an
inttoptr constant expression base.

To compensate for this, make sure that inttoptr constant expressions
are treated as escape sources, just like inttoptr instructions.
This ensures that the optimization can still be applied if the
local object is non-escaping. This is sufficient to still optimize
the original motivation case from c53e2ec.
@llvmbot
Copy link
Collaborator

llvmbot commented Jan 3, 2024

@llvm/pr-subscribers-llvm-analysis

Author: Nikita Popov (nikic)

Changes

BasicAA currently says that any Constant cannot alias an identified local object. This is not correct if the local object escaped, as it's possible to create a pointer to the escaped object using an inttoptr constant expression base.

To compensate for this, make sure that inttoptr constant expressions are treated as escape sources, just like inttoptr instructions. This ensures that the optimization can still be applied if the local object is non-escaping. This is sufficient to still optimize the original motivation case from c53e2ec.

Fixes #76789.


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

4 Files Affected:

  • (modified) llvm/include/llvm/Analysis/AliasAnalysis.h (+2)
  • (modified) llvm/lib/Analysis/AliasAnalysis.cpp (+5)
  • (modified) llvm/lib/Analysis/BasicAliasAnalysis.cpp (+9-10)
  • (modified) llvm/test/Analysis/BasicAA/inttoptr_constexpr.ll (+3-3)
diff --git a/llvm/include/llvm/Analysis/AliasAnalysis.h b/llvm/include/llvm/Analysis/AliasAnalysis.h
index e1cfb025fb6580..d6f732d35fd4cd 100644
--- a/llvm/include/llvm/Analysis/AliasAnalysis.h
+++ b/llvm/include/llvm/Analysis/AliasAnalysis.h
@@ -154,6 +154,8 @@ struct CaptureInfo {
 
   /// Check whether Object is not captured before instruction I. If OrAt is
   /// true, captures by instruction I itself are also considered.
+  ///
+  /// If I is nullptr, then captures at any point will be considered.
   virtual bool isNotCapturedBefore(const Value *Object, const Instruction *I,
                                    bool OrAt) = 0;
 };
diff --git a/llvm/lib/Analysis/AliasAnalysis.cpp b/llvm/lib/Analysis/AliasAnalysis.cpp
index da18279ae9b93d..6eaaad5f332eb9 100644
--- a/llvm/lib/Analysis/AliasAnalysis.cpp
+++ b/llvm/lib/Analysis/AliasAnalysis.cpp
@@ -883,6 +883,11 @@ bool llvm::isEscapeSource(const Value *V) {
   if (isa<IntToPtrInst>(V))
     return true;
 
+  // Same for inttoptr constant expressions.
+  if (auto *CE = dyn_cast<ConstantExpr>(V))
+    if (CE->getOpcode() == Instruction::IntToPtr)
+      return true;
+
   return false;
 }
 
diff --git a/llvm/lib/Analysis/BasicAliasAnalysis.cpp b/llvm/lib/Analysis/BasicAliasAnalysis.cpp
index 3de147368f2346..410e17d5564269 100644
--- a/llvm/lib/Analysis/BasicAliasAnalysis.cpp
+++ b/llvm/lib/Analysis/BasicAliasAnalysis.cpp
@@ -215,7 +215,7 @@ bool EarliestEscapeInfo::isNotCapturedBefore(const Value *Object,
   auto Iter = EarliestEscapes.insert({Object, nullptr});
   if (Iter.second) {
     Instruction *EarliestCapture = FindEarliestCapture(
-        Object, *const_cast<Function *>(I->getFunction()),
+        Object, *const_cast<Function *>(DT.getRoot()->getParent()),
         /*ReturnCaptures=*/false, /*StoreCaptures=*/true, DT);
     if (EarliestCapture) {
       auto Ins = Inst2Obj.insert({EarliestCapture, {}});
@@ -228,6 +228,10 @@ bool EarliestEscapeInfo::isNotCapturedBefore(const Value *Object,
   if (!Iter.first->second)
     return true;
 
+  // No context instruction means any use is capturing.
+  if (!I)
+    return false;
+
   if (I == Iter.first->second) {
     if (OrAt)
       return false;
@@ -1504,11 +1508,6 @@ AliasResult BasicAAResult::aliasCheck(const Value *V1, LocationSize V1Size,
     if (isIdentifiedObject(O1) && isIdentifiedObject(O2))
       return AliasResult::NoAlias;
 
-    // Constant pointers can't alias with non-const isIdentifiedObject objects.
-    if ((isa<Constant>(O1) && isIdentifiedObject(O2) && !isa<Constant>(O2)) ||
-        (isa<Constant>(O2) && isIdentifiedObject(O1) && !isa<Constant>(O1)))
-      return AliasResult::NoAlias;
-
     // Function arguments can't alias with things that are known to be
     // unambigously identified at the function level.
     if ((isa<Argument>(O1) && isIdentifiedFunctionLocal(O2)) ||
@@ -1524,11 +1523,11 @@ AliasResult BasicAAResult::aliasCheck(const Value *V1, LocationSize V1Size,
     // temporary store the nocapture argument's value in a temporary memory
     // location if that memory location doesn't escape. Or it may pass a
     // nocapture value to other functions as long as they don't capture it.
-    if (isEscapeSource(O1) &&
-        AAQI.CI->isNotCapturedBefore(O2, cast<Instruction>(O1), /*OrAt*/ true))
+    if (isEscapeSource(O1) && AAQI.CI->isNotCapturedBefore(
+                                  O2, dyn_cast<Instruction>(O1), /*OrAt*/ true))
       return AliasResult::NoAlias;
-    if (isEscapeSource(O2) &&
-        AAQI.CI->isNotCapturedBefore(O1, cast<Instruction>(O2), /*OrAt*/ true))
+    if (isEscapeSource(O2) && AAQI.CI->isNotCapturedBefore(
+                                  O1, dyn_cast<Instruction>(O2), /*OrAt*/ true))
       return AliasResult::NoAlias;
   }
 
diff --git a/llvm/test/Analysis/BasicAA/inttoptr_constexpr.ll b/llvm/test/Analysis/BasicAA/inttoptr_constexpr.ll
index afe3602d1c7e5e..6d4ad08d1621b5 100644
--- a/llvm/test/Analysis/BasicAA/inttoptr_constexpr.ll
+++ b/llvm/test/Analysis/BasicAA/inttoptr_constexpr.ll
@@ -1,6 +1,6 @@
 ; RUN: opt -passes=aa-eval -print-all-alias-modref-info -disable-output < %s 2>&1 | FileCheck %s
 
-; CHECK: NoAlias: i8* %a, i8* %gep
+; CHECK: MayAlias: i8* %a, i8* %gep
 define void @inttoptr_alloca() {
   %a = alloca i8
   %a.int = ptrtoint ptr %a to i64
@@ -11,7 +11,7 @@ define void @inttoptr_alloca() {
   ret void
 }
 
-; CHECK: NoAlias: i8* %a, i8* %gep
+; CHECK: MayAlias: i8* %a, i8* %gep
 define void @inttoptr_alloca_unknown_relation(i64 %offset) {
   %a = alloca i8
   %a.int = ptrtoint ptr %a to i64
@@ -30,7 +30,7 @@ define void @inttoptr_alloca_noescape(i64 %offset) {
   ret void
 }
 
-; CHECK: NoAlias: i8* %a, i8* %gep
+; CHECK: MayAlias: i8* %a, i8* %gep
 define void @inttoptr_noalias(ptr noalias %a) {
   %a.int = ptrtoint ptr %a to i64
   %a.int.1 = add i64 %a.int, 1

@nikic
Copy link
Contributor Author

nikic commented Jan 10, 2024

ping

Copy link
Contributor

@fhahn fhahn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

@nikic nikic merged commit 5f57ad8 into llvm:main Jan 17, 2024
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Wrong code at -O1 on x86_64-linux_gnu since LLVM-13
3 participants