[llvm] 70e3c9a - [BasicAA] Always strip single-argument phi nodes

Nikita Popov via llvm-commits llvm-commits at lists.llvm.org
Thu Feb 18 14:08:02 PST 2021


Author: Nikita Popov
Date: 2021-02-18T23:07:50+01:00
New Revision: 70e3c9a8b6684c49576aee38b5c90b18fdf00d2c

URL: https://github.com/llvm/llvm-project/commit/70e3c9a8b6684c49576aee38b5c90b18fdf00d2c
DIFF: https://github.com/llvm/llvm-project/commit/70e3c9a8b6684c49576aee38b5c90b18fdf00d2c.diff

LOG: [BasicAA] Always strip single-argument phi nodes

We can always look through single-argument (LCSSA) phi nodes when
performing alias analysis. getUnderlyingObject() already does this,
but stripPointerCastsAndInvariantGroups() does not. We still look
through these phi nodes with the usual aliasPhi() logic, but
sometimes get sub-optimal results due to the restrictions on value
equivalence when looking through arbitrary phi nodes. I think it's
generally beneficial to keep the underlying object logic and the
pointer cast stripping logic in sync, insofar as it is possible.

With this patch we get marginally better results:

  aa.NumMayAlias | 5010069 | 5009861
  aa.NumMustAlias | 347518 | 347674
  aa.NumNoAlias | 27201336 | 27201528
  ...
  licm.NumPromoted | 1293 | 1296

I've renamed the relevant strip method to stripPointerCastsForAliasAnalysis(),
as we're past the point where we can explicitly spell out everything
that's getting stripped.

Differential Revision: https://reviews.llvm.org/D96668

Added: 
    

Modified: 
    llvm/include/llvm/IR/Value.h
    llvm/lib/Analysis/BasicAliasAnalysis.cpp
    llvm/lib/Analysis/GlobalsModRef.cpp
    llvm/lib/IR/Value.cpp
    llvm/lib/Target/AMDGPU/AMDGPUAliasAnalysis.cpp
    llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
    llvm/test/Analysis/BasicAA/phi-aa.ll

Removed: 
    


################################################################################
diff  --git a/llvm/include/llvm/IR/Value.h b/llvm/include/llvm/IR/Value.h
index 01dfdf35e681..5a7e90aeb0f6 100644
--- a/llvm/include/llvm/IR/Value.h
+++ b/llvm/include/llvm/IR/Value.h
@@ -654,15 +654,16 @@ class Value {
                                    ->stripPointerCastsSameRepresentation());
   }
 
-  /// Strip off pointer casts, all-zero GEPs and invariant group info.
+  /// Strip off pointer casts, all-zero GEPs, single-argument phi nodes and
+  /// invariant group info.
   ///
   /// Returns the original uncasted value.  If this is called on a non-pointer
   /// value, it returns 'this'. This function should be used only in
   /// Alias analysis.
-  const Value *stripPointerCastsAndInvariantGroups() const;
-  Value *stripPointerCastsAndInvariantGroups() {
+  const Value *stripPointerCastsForAliasAnalysis() const;
+  Value *stripPointerCastsForAliasAnalysis() {
     return const_cast<Value *>(static_cast<const Value *>(this)
-                                   ->stripPointerCastsAndInvariantGroups());
+                                   ->stripPointerCastsForAliasAnalysis());
   }
 
   /// Strip off pointer casts and all-constant inbounds GEPs.

diff  --git a/llvm/lib/Analysis/BasicAliasAnalysis.cpp b/llvm/lib/Analysis/BasicAliasAnalysis.cpp
index ac3a32742f14..81996f3f8e37 100644
--- a/llvm/lib/Analysis/BasicAliasAnalysis.cpp
+++ b/llvm/lib/Analysis/BasicAliasAnalysis.cpp
@@ -1470,8 +1470,8 @@ AliasResult BasicAAResult::aliasCheck(const Value *V1, LocationSize V1Size,
     return NoAlias;
 
   // Strip off any casts if they exist.
-  V1 = V1->stripPointerCastsAndInvariantGroups();
-  V2 = V2->stripPointerCastsAndInvariantGroups();
+  V1 = V1->stripPointerCastsForAliasAnalysis();
+  V2 = V2->stripPointerCastsForAliasAnalysis();
 
   // If V1 or V2 is undef, the result is NoAlias because we can always pick a
   // value for undef that aliases nothing in the program.

diff  --git a/llvm/lib/Analysis/GlobalsModRef.cpp b/llvm/lib/Analysis/GlobalsModRef.cpp
index 145baf82b65b..aafc19ac6ef1 100644
--- a/llvm/lib/Analysis/GlobalsModRef.cpp
+++ b/llvm/lib/Analysis/GlobalsModRef.cpp
@@ -828,9 +828,9 @@ AliasResult GlobalsAAResult::alias(const MemoryLocation &LocA,
                                    AAQueryInfo &AAQI) {
   // Get the base object these pointers point to.
   const Value *UV1 =
-      getUnderlyingObject(LocA.Ptr->stripPointerCastsAndInvariantGroups());
+      getUnderlyingObject(LocA.Ptr->stripPointerCastsForAliasAnalysis());
   const Value *UV2 =
-      getUnderlyingObject(LocB.Ptr->stripPointerCastsAndInvariantGroups());
+      getUnderlyingObject(LocB.Ptr->stripPointerCastsForAliasAnalysis());
 
   // If either of the underlying values is a global, they may be non-addr-taken
   // globals, which we can answer queries about.

diff  --git a/llvm/lib/IR/Value.cpp b/llvm/lib/IR/Value.cpp
index 572f37a32410..92ffae18ae6f 100644
--- a/llvm/lib/IR/Value.cpp
+++ b/llvm/lib/IR/Value.cpp
@@ -551,7 +551,7 @@ enum PointerStripKind {
   PSK_ZeroIndices,
   PSK_ZeroIndicesAndAliases,
   PSK_ZeroIndicesSameRepresentation,
-  PSK_ZeroIndicesAndInvariantGroups,
+  PSK_ForAliasAnalysis,
   PSK_InBoundsConstantIndices,
   PSK_InBounds
 };
@@ -577,7 +577,7 @@ static const Value *stripPointerCastsAndOffsets(
       case PSK_ZeroIndices:
       case PSK_ZeroIndicesAndAliases:
       case PSK_ZeroIndicesSameRepresentation:
-      case PSK_ZeroIndicesAndInvariantGroups:
+      case PSK_ForAliasAnalysis:
         if (!GEP->hasAllZeroIndices())
           return V;
         break;
@@ -602,6 +602,9 @@ static const Value *stripPointerCastsAndOffsets(
       V = cast<Operator>(V)->getOperand(0);
     } else if (StripKind == PSK_ZeroIndicesAndAliases && isa<GlobalAlias>(V)) {
       V = cast<GlobalAlias>(V)->getAliasee();
+    } else if (StripKind == PSK_ForAliasAnalysis && isa<PHINode>(V) &&
+               cast<PHINode>(V)->getNumIncomingValues() == 1) {
+      V = cast<PHINode>(V)->getIncomingValue(0);
     } else {
       if (const auto *Call = dyn_cast<CallBase>(V)) {
         if (const Value *RV = Call->getReturnedArgOperand()) {
@@ -611,7 +614,7 @@ static const Value *stripPointerCastsAndOffsets(
         // The result of launder.invariant.group must alias it's argument,
         // but it can't be marked with returned attribute, that's why it needs
         // special case.
-        if (StripKind == PSK_ZeroIndicesAndInvariantGroups &&
+        if (StripKind == PSK_ForAliasAnalysis &&
             (Call->getIntrinsicID() == Intrinsic::launder_invariant_group ||
              Call->getIntrinsicID() == Intrinsic::strip_invariant_group)) {
           V = Call->getArgOperand(0);
@@ -643,8 +646,8 @@ const Value *Value::stripInBoundsConstantOffsets() const {
   return stripPointerCastsAndOffsets<PSK_InBoundsConstantIndices>(this);
 }
 
-const Value *Value::stripPointerCastsAndInvariantGroups() const {
-  return stripPointerCastsAndOffsets<PSK_ZeroIndicesAndInvariantGroups>(this);
+const Value *Value::stripPointerCastsForAliasAnalysis() const {
+  return stripPointerCastsAndOffsets<PSK_ForAliasAnalysis>(this);
 }
 
 const Value *Value::stripAndAccumulateConstantOffsets(

diff  --git a/llvm/lib/Target/AMDGPU/AMDGPUAliasAnalysis.cpp b/llvm/lib/Target/AMDGPU/AMDGPUAliasAnalysis.cpp
index 0ed89e9ca8d6..31e17672c4c2 100644
--- a/llvm/lib/Target/AMDGPU/AMDGPUAliasAnalysis.cpp
+++ b/llvm/lib/Target/AMDGPU/AMDGPUAliasAnalysis.cpp
@@ -87,7 +87,7 @@ AliasResult AMDGPUAAResult::alias(const MemoryLocation &LocA,
   if (asA == AMDGPUAS::FLAT_ADDRESS &&
       (asB == AMDGPUAS::LOCAL_ADDRESS || asB == AMDGPUAS::PRIVATE_ADDRESS)) {
     const auto *ObjA =
-        getUnderlyingObject(A.Ptr->stripPointerCastsAndInvariantGroups());
+        getUnderlyingObject(A.Ptr->stripPointerCastsForAliasAnalysis());
     if (const LoadInst *LI = dyn_cast<LoadInst>(ObjA)) {
       // If a generic pointer is loaded from the constant address space, it
       // could only be a GLOBAL or CONSTANT one as that address space is soley

diff  --git a/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp b/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
index 99524f5678ce..df8fade74174 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
@@ -390,7 +390,13 @@ static Instruction *simplifyInvariantGroupIntrinsic(IntrinsicInst &II,
                                                     InstCombinerImpl &IC) {
   auto *Arg = II.getArgOperand(0);
   auto *StrippedArg = Arg->stripPointerCasts();
-  auto *StrippedInvariantGroupsArg = Arg->stripPointerCastsAndInvariantGroups();
+  auto *StrippedInvariantGroupsArg = StrippedArg;
+  while (auto *Intr = dyn_cast<IntrinsicInst>(StrippedInvariantGroupsArg)) {
+    if (Intr->getIntrinsicID() != Intrinsic::launder_invariant_group &&
+        Intr->getIntrinsicID() != Intrinsic::strip_invariant_group)
+      break;
+    StrippedInvariantGroupsArg = Intr->getArgOperand(0)->stripPointerCasts();
+  }
   if (StrippedArg == StrippedInvariantGroupsArg)
     return nullptr; // No launders/strips to remove.
 

diff  --git a/llvm/test/Analysis/BasicAA/phi-aa.ll b/llvm/test/Analysis/BasicAA/phi-aa.ll
index 39a31808296a..357fd498c1a2 100644
--- a/llvm/test/Analysis/BasicAA/phi-aa.ll
+++ b/llvm/test/Analysis/BasicAA/phi-aa.ll
@@ -199,9 +199,9 @@ for.body:                                         ; preds = %for.body, %entry
 } 
 
 ; CHECK-LABEL: single_arg_phi
-; CHECK: MayAlias: i32* %ptr, i32* %ptr.next
+; CHECK: NoAlias: i32* %ptr, i32* %ptr.next
+; CHECK: MustAlias: i32* %ptr, i32* %ptr.phi
 ; CHECK: MustAlias: i32* %ptr.next, i32* %ptr.next.phi
-; TODO: The first one could be MustAlias as well.
 define void @single_arg_phi(i32* %ptr.base) {
 entry:
   br label %loop


        


More information about the llvm-commits mailing list