[Attributor] Change AANoUndef not to deduce noundef for dead returned positions

[okura]

Summary

When a returned position is dead, returned values will be replaced with undef values by AAIsDead.
Therefore we should not deduce noundef for the position. Otherwise, AAUndefinedBehavior caught that and replace them with unreachable incorrectly.

[sstefan1]

LGTM, thanks!

@jdoerfert ?

[okura]

When I fix the identification of AANoUndef in identifyDefaultAbstractAttributes, I noticed that noundef should not be deduced for dead argument and dead call site argument position too. (I found wrong transformation in e.g. callbacks.ll)
Not to deduce for dead positions, we should not indicate optimistic fixpoint in initialize because once fixpoint is indicated we cannot change the state. However, it weakens the deduction significantly.
So I want to suggest that we check not only noundef but also liveness in AAUndefinedBehavior.

[jdoerfert]

In D86396#2236597, @okura wrote:

When I fix the identification of AANoUndef in identifyDefaultAbstractAttributes, I noticed that noundef should not be deduced for dead argument and dead call site argument position too. (I found wrong transformation in e.g. callbacks.ll)
Not to deduce for dead positions, we should not indicate optimistic fixpoint in initialize because once fixpoint is indicated we cannot change the state. However, it weakens the deduction significantly.
So I want to suggest that we check not only noundef but also liveness in AAUndefinedBehavior.

I don't understand the comment.

Should we check in the manifest of AANoUndef if the position is dead and not manifest it? I'm not sure we necessarily call updateImpl before with a known dead state, so this approach might not always work.

[jdoerfert]

(change status so it shows up on my review list again)

[okura]

In D86396#2236684, @jdoerfert wrote:

In D86396#2236597, @okura wrote:

When I fix the identification of AANoUndef in identifyDefaultAbstractAttributes, I noticed that noundef should not be deduced for dead argument and dead call site argument position too. (I found wrong transformation in e.g. callbacks.ll)
Not to deduce for dead positions, we should not indicate optimistic fixpoint in initialize because once fixpoint is indicated we cannot change the state. However, it weakens the deduction significantly.
So I want to suggest that we check not only noundef but also liveness in AAUndefinedBehavior.

I don't understand the comment.
Should we check in the manifest of AANoUndef if the position is dead and not manifest it?

I investigated the problem seen in the module slice patch again. And I realize now that checking liveness in manifestation is sufficient to settle the problem.
I misunderstood we need more things to do so. I was overthinking, sorry.

I will abandon this revision because I have made D86565 for the problem.

llvm/lib/Transforms/IPO/AttributorAttributes.cpp

struct AANoUndefFloating : public AANoUndefImpl {
   }
 
   /// See AbstractAttribute::trackStatistics()
   void trackStatistics() const override { STATS_DECLTRACK_FNRET_ATTR(noundef) }
 };
 
 struct AANoUndefReturned final
     : AAReturnedFromReturnedValues<AANoUndef, AANoUndefImpl> {
-  AANoUndefReturned(const IRPosition &IRP, Attributor &A)
-      : AAReturnedFromReturnedValues<AANoUndef, AANoUndefImpl>(IRP, A) {}
+  using Base = AAReturnedFromReturnedValues<AANoUndef, AANoUndefImpl>;
+  AANoUndefReturned(const IRPosition &IRP, Attributor &A) : Base(IRP, A) {}
+
+  /// See AbstractAttribute::updateImpl(...).
+  ChangeStatus updateImpl(Attributor &A) override {
+    auto &LivenessAA = A.getAAFor<AAIsDead>(*this, getIRPosition());
+    // When the returned position is dead, returned values will be replaced with
+    // undef values by AAIsDead. So we don't deduce noundef for this position.
+    if (LivenessAA.isKnown())
+      return indicatePessimisticFixpoint();
+    return Base::updateImpl(A);
+  }
 
   /// See AbstractAttribute::trackStatistics()
   void trackStatistics() const override { STATS_DECLTRACK_FNRET_ATTR(noundef) }
 };
 
 struct AANoUndefArgument final
     : AAArgumentFromCallSiteArguments<AANoUndef, AANoUndefImpl> {
   AANoUndefArgument(const IRPosition &IRP, Attributor &A)

llvm/test/Transforms/Attributor/noundef.ll

 ; CHECK-NEXT:    call void @bar(i32* noundef nonnull align 4 dereferenceable(4) [[X]])
 ; CHECK-NEXT:    ret void
 ;
   %x = alloca i32
   call void @unknown()
   call void @bar(i32* %x)
   ret void
 }
+
+define internal i8* @returned_dead() {
+; CHECK-LABEL: define internal noalias align 536870912 i8* @returned_dead(
+; CHECK-NEXT:    call void @unknown()
+; CHECK-NEXT:    ret i8* undef
+;
+  call void @unknown()
+  ret i8* null
+}
+
+define void @caller() {
+; CHECK-LABEL: @caller(
+; CHECK-NEXT:    [[TMP1:%.*]] = call i8* @returned_dead()
+; CHECK-NEXT:    ret void
+;
+  call i8* @returned_dead()
+  ret void
+}