Incorporated feedback

Author: zGoldthorpe

llvm/include/llvm/Transforms/IPO/Attributor.h

@@ -6298,6 +6298,7 @@ struct AAInvariantLoadPointer : public AbstractAttribute {
   static bool isValidIRPositionForInit(Attributor &A, const IRPosition &IRP) {
     if (!IRP.getAssociatedType()->isPointerTy())
       return false;
+
     return AbstractAttribute::isValidIRPositionForInit(A, IRP);
   }
 

llvm/lib/Transforms/IPO/AttributorAttributes.cpp

@@ -12579,7 +12579,7 @@ struct AAInvariantLoadPointerImpl
 
   ChangeStatus updateImpl(Attributor &A) override {
     if (isKnownInvariant())
-      return ChangeStatus::UNCHANGED;
+      return indicateOptimisticFixpoint();
 
     ChangeStatus Changed = ChangeStatus::UNCHANGED;
 
@@ -12605,15 +12605,13 @@ struct AAInvariantLoadPointerImpl
     const auto *AUO = A.getOrCreateAAFor<AAUnderlyingObjects>(
         getIRPosition(), this, DepClassTy::REQUIRED);
 
-    if (!AUO->forallUnderlyingObjects(IsInvariantLoadIfPointer)) {
-      removeAssumedBits(IS_INVARIANT);
-      return ChangeStatus::CHANGED;
-    }
+    if (!AUO->forallUnderlyingObjects(IsInvariantLoadIfPointer))
+      return indicatePessimisticFixpoint();
 
     if (!UsedAssumedInformation) {
       // pointer is known (not assumed) to be invariant
       addKnownBits(IS_INVARIANT);
-      return ChangeStatus::CHANGED;
+      return indicateOptimisticFixpoint() | Changed;
     }
 
     return Changed;
@@ -12671,53 +12669,90 @@ struct AAInvariantLoadPointerImpl
     if (isKnown(IS_NOALIAS) || !isAssumed(IS_NOALIAS))
       return ChangeStatus::UNCHANGED;
 
-    const auto *ANoAlias = A.getOrCreateAAFor<AANoAlias>(getIRPosition(), this,
-                                                         DepClassTy::REQUIRED);
-    if (!ANoAlias)
-      return tryInferNoAlias(A);
+    const auto *F = getAssociatedFunction();
 
-    if (!ANoAlias->isAssumedNoAlias()) {
+    if (F && isCallableCC(F->getCallingConv())) {
+      // program-wide alias information cannot be inferred
       removeAssumedBits(IS_NOALIAS);
       return ChangeStatus::CHANGED;
     }
-    if (ANoAlias->isKnownNoAlias())
-      addKnownBits(IS_NOALIAS);
 
-    return ChangeStatus::UNCHANGED;
-  }
+    // try to use AANoAlias
+    if (const auto *ANoAlias = A.getOrCreateAAFor<AANoAlias>(
+            getIRPosition(), this, DepClassTy::REQUIRED)) {
+      if (ANoAlias->isKnownNoAlias()) {
+        addKnownBits(IS_NOALIAS);
+        return ChangeStatus::UNCHANGED;
+      }
+
+      if (!ANoAlias->isAssumedNoAlias()) {
+        removeAssumedBits(IS_NOALIAS);
+        return ChangeStatus::CHANGED;
+      }
+
+      return ChangeStatus::UNCHANGED;
+    }
+
+    // if the function is not callable, try to infer noalias from argument
+    // attribute, since it is applicable for the duration of the function
+    if (const auto *Arg = getAssociatedArgument()) {
+      if (Arg->hasNoAliasAttr()) {
+        addKnownBits(IS_NOALIAS);
+        return ChangeStatus::UNCHANGED;
+      }
+
+      // noalias information is not provided, and cannot be inferred,
+      // so we conservatively assume the pointer aliases.
+      removeAssumedBits(IS_NOALIAS);
+      return ChangeStatus::CHANGED;
+    }
 
-  /// Fallback method if updateNoAlias fails to infer noalias information from
-  /// AANoAlias.
-  virtual ChangeStatus tryInferNoAlias(Attributor &A) {
     return ChangeStatus::UNCHANGED;
   }
 
   ChangeStatus updateReadOnly(Attributor &A) {
     if (isKnown(IS_READONLY) || !isAssumed(IS_READONLY))
       return ChangeStatus::UNCHANGED;
 
-    // AAMemoryBehavior may crash if value is global
-    if (!getAssociatedFunction())
-      return tryInferReadOnly(A);
+    const auto *F = getAssociatedFunction();
 
-    const auto *AMemoryBehavior = A.getOrCreateAAFor<AAMemoryBehavior>(
-        getIRPosition(), this, DepClassTy::REQUIRED);
-    if (!AMemoryBehavior)
-      return tryInferReadOnly(A);
+    if (!F)
+      return ChangeStatus::UNCHANGED;
 
-    if (!AMemoryBehavior->isAssumedReadOnly()) {
+    if (isCallableCC(F->getCallingConv())) {
+      // readonly attribute is only useful if applicable program-wide
       removeAssumedBits(IS_READONLY);
       return ChangeStatus::CHANGED;
     }
-    if (AMemoryBehavior->isKnownReadOnly())
-      addKnownBits(IS_READONLY);
 
-    return ChangeStatus::UNCHANGED;
-  }
+    // try to use AAMemoryBehavior to infer readonly attribute
+    if (const auto *AMemoryBehavior = A.getOrCreateAAFor<AAMemoryBehavior>(
+            getIRPosition(), this, DepClassTy::REQUIRED)) {
+      if (!AMemoryBehavior->isAssumedReadOnly()) {
+        removeAssumedBits(IS_READONLY);
+        return ChangeStatus::CHANGED;
+      }
+
+      if (AMemoryBehavior->isKnownReadOnly()) {
+        addKnownBits(IS_READONLY);
+        return ChangeStatus::UNCHANGED;
+      }
+
+      return ChangeStatus::UNCHANGED;
+    }
+
+    if (const auto *Arg = getAssociatedArgument()) {
+      if (Arg->onlyReadsMemory()) {
+        addKnownBits(IS_READONLY);
+        return ChangeStatus::UNCHANGED;
+      }
+
+      // readonly information is not provided, and cannot be inferred from
+      // AAMemoryBehavior
+      removeAssumedBits(IS_READONLY);
+      return ChangeStatus::CHANGED;
+    }
 
-  /// Fallback method if updateReadOnly fails to infer readonly information from
-  /// AAMemoryBehavior.
-  virtual ChangeStatus tryInferReadOnly(Attributor &A) {
     return ChangeStatus::UNCHANGED;
   }
 };
@@ -12741,33 +12776,6 @@ struct AAInvariantLoadPointerCallSiteReturned final
 struct AAInvariantLoadPointerArgument final : AAInvariantLoadPointerImpl {
   AAInvariantLoadPointerArgument(const IRPosition &IRP, Attributor &A)
       : AAInvariantLoadPointerImpl(IRP, A) {}
-
-protected:
-  ChangeStatus tryInferNoAlias(Attributor &A) override {
-    const auto *Arg = getAssociatedArgument();
-    if (Arg->hasNoAliasAttr()) {
-      addKnownBits(IS_NOALIAS);
-      return ChangeStatus::UNCHANGED;
-    }
-
-    // noalias information is not provided, and cannot be inferred from
-    // AANoAlias
-    removeAssumedBits(IS_NOALIAS);
-    return ChangeStatus::CHANGED;
-  }
-
-  ChangeStatus tryInferReadOnly(Attributor &A) override {
-    const auto *Arg = getAssociatedArgument();
-    if (Arg->onlyReadsMemory()) {
-      addKnownBits(IS_READONLY);
-      return ChangeStatus::UNCHANGED;
-    }
-
-    // readonly information is not provided, and cannot be inferred from
-    // AAMemoryBehavior
-    removeAssumedBits(IS_READONLY);
-    return ChangeStatus::CHANGED;
-  }
 };
 
 struct AAInvariantLoadPointerCallSiteArgument final

llvm/test/Transforms/Attributor/multiple-offsets-pointer-info.ll

@@ -10,7 +10,7 @@ define i8 @select_offsets_simplifiable_1(i1 %cnd1, i1 %cnd2) {
 ; CHECK-LABEL: define {{[^@]+}}@select_offsets_simplifiable_1
 ; CHECK-SAME: (i1 [[CND1:%.*]], i1 [[CND2:%.*]]) {
 ; CHECK-NEXT:  entry:
-; CHECK-NEXT:    [[BYTES:%.*]] = call noalias ptr @calloc(i64 noundef 1024, i64 noundef 1)
+; CHECK-NEXT:    [[BYTES:%.*]] = call ptr @calloc(i64 noundef 1024, i64 noundef 1)
 ; CHECK-NEXT:    [[GEP23:%.*]] = getelementptr inbounds [1024 x i8], ptr [[BYTES]], i64 0, i64 23
 ; CHECK-NEXT:    store i8 23, ptr [[GEP23]], align 4
 ; CHECK-NEXT:    [[GEP29:%.*]] = getelementptr inbounds [1024 x i8], ptr [[BYTES]], i64 0, i64 29
@@ -190,7 +190,7 @@ define i8 @select_offsets_not_simplifiable_3(i1 %cnd1, i1 %cnd2) {
 ; CHECK-LABEL: define {{[^@]+}}@select_offsets_not_simplifiable_3
 ; CHECK-SAME: (i1 [[CND1:%.*]], i1 [[CND2:%.*]]) {
 ; CHECK-NEXT:  entry:
-; CHECK-NEXT:    [[BYTES:%.*]] = call noalias ptr @calloc(i64 noundef 1024, i64 noundef 1)
+; CHECK-NEXT:    [[BYTES:%.*]] = call ptr @calloc(i64 noundef 1024, i64 noundef 1)
 ; CHECK-NEXT:    [[SEL0:%.*]] = select i1 [[CND1]], i64 23, i64 29
 ; CHECK-NEXT:    [[SEL1:%.*]] = select i1 [[CND2]], i64 [[SEL0]], i64 7
 ; CHECK-NEXT:    [[GEP_SEL:%.*]] = getelementptr inbounds [1024 x i8], ptr [[BYTES]], i64 0, i64 [[SEL1]]
@@ -214,7 +214,7 @@ define i8 @select_offsets_not_simplifiable_4(i1 %cnd1, i1 %cnd2) {
 ; CHECK-LABEL: define {{[^@]+}}@select_offsets_not_simplifiable_4
 ; CHECK-SAME: (i1 [[CND1:%.*]], i1 [[CND2:%.*]]) {
 ; CHECK-NEXT:  entry:
-; CHECK-NEXT:    [[BYTES:%.*]] = call noalias ptr @calloc(i64 noundef 1024, i64 noundef 1)
+; CHECK-NEXT:    [[BYTES:%.*]] = call ptr @calloc(i64 noundef 1024, i64 noundef 1)
 ; CHECK-NEXT:    [[SEL0:%.*]] = select i1 [[CND1]], i64 23, i64 29
 ; CHECK-NEXT:    [[SEL1:%.*]] = select i1 [[CND2]], i64 [[SEL0]], i64 7
 ; CHECK-NEXT:    [[GEP_SEL:%.*]] = getelementptr inbounds [1024 x i8], ptr [[BYTES]], i64 0, i64 [[SEL1]]
@@ -445,7 +445,7 @@ define i8 @phi_gep_not_simplifiable_2(i1 %cnd1, i1 %cnd2) {
 ; CHECK-LABEL: define {{[^@]+}}@phi_gep_not_simplifiable_2
 ; CHECK-SAME: (i1 [[CND1:%.*]], i1 [[CND2:%.*]]) {
 ; CHECK-NEXT:  entry:
-; CHECK-NEXT:    [[BYTES:%.*]] = call noalias ptr @calloc(i64 noundef 1024, i64 noundef 1)
+; CHECK-NEXT:    [[BYTES:%.*]] = call ptr @calloc(i64 noundef 1024, i64 noundef 1)
 ; CHECK-NEXT:    [[GEP23:%.*]] = getelementptr inbounds [1024 x i8], ptr [[BYTES]], i64 0, i64 23
 ; CHECK-NEXT:    br i1 [[CND1]], label [[THEN:%.*]], label [[ELSE:%.*]]
 ; CHECK:       then: