[llvm] 54ec9b5 - [Attributor] Unify handling of must-tail calls

[Johannes Doerfert via llvm-commits]

Author: Johannes Doerfert
Date: 2020-03-23T10:45:24-05:00
New Revision: 54ec9b54f6bb7d580447570fb91451f663113d35

URL: https://github.com/llvm/llvm-project/commit/54ec9b54f6bb7d580447570fb91451f663113d35
DIFF: https://github.com/llvm/llvm-project/commit/54ec9b54f6bb7d580447570fb91451f663113d35.diff

LOG: [Attributor] Unify handling of must-tail calls

We special cased must-tail calls all over the place because they cannot
be modified as other calls can be. However, we already centralized the
modification API so we can centralize the handling as well. This
simplifies the code and allows to remove must-tail calls completely.

Added: 
    

Modified: 
    llvm/lib/Transforms/IPO/Attributor.cpp
    llvm/test/Transforms/Attributor/IPConstantProp/musttail-call.ll

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Transforms/IPO/Attributor.cpp b/llvm/lib/Transforms/IPO/Attributor.cpp
index 231b42d2bf6c..fb860bdf7a01 100644
--- a/llvm/lib/Transforms/IPO/Attributor.cpp
+++ b/llvm/lib/Transforms/IPO/Attributor.cpp
@@ -1317,7 +1317,7 @@ ChangeStatus AAReturnedValuesImpl::manifest(Attributor &A) {
 
   // Callback to replace the uses of CB with the constant C.
   auto ReplaceCallSiteUsersWith = [&A](CallBase &CB, Constant &C) {
-    if (CB.getNumUses() == 0 || CB.isMustTailCall())
+    if (CB.getNumUses() == 0)
       return ChangeStatus::UNCHANGED;
     if (A.changeValueAfterManifest(CB, C))
       return ChangeStatus::CHANGED;
@@ -3207,14 +3207,6 @@ struct AAIsDeadCallSiteReturned : public AAIsDeadFloating {
     return Changed;
   }
 
-  /// See AbstractAttribute::manifest(...).
-  ChangeStatus manifest(Attributor &A) override {
-    if (auto *CI = dyn_cast<CallInst>(&getAssociatedValue()))
-      if (CI->isMustTailCall())
-        return ChangeStatus::UNCHANGED;
-    return AAIsDeadFloating::manifest(A);
-  }
-
   /// See AbstractAttribute::trackStatistics()
   void trackStatistics() const override {
     if (IsAssumedSideEffectFree)
@@ -3264,9 +3256,6 @@ struct AAIsDeadReturned : public AAIsDeadValueImpl {
     UndefValue &UV = *UndefValue::get(getAssociatedFunction()->getReturnType());
     auto RetInstPred = [&](Instruction &I) {
       ReturnInst &RI = cast<ReturnInst>(I);
-      if (auto *CI = dyn_cast<CallInst>(RI.getReturnValue()))
-        if (CI->isMustTailCall())
-          return true;
       if (!isa<UndefValue>(RI.getReturnValue()))
         AnyChange |= A.changeUseAfterManifest(RI.getOperandUse(0), UV);
       return true;
@@ -4855,9 +4844,6 @@ struct AAValueSimplifyReturned : AAValueSimplifyImpl {
             // We can replace the AssociatedValue with the constant.
             if (&V == C || V.getType() != C->getType() || isa<UndefValue>(V))
               return true;
-            if (auto *CI = dyn_cast<CallInst>(&V))
-              if (CI->isMustTailCall())
-                return true;
 
             for (ReturnInst *RI : RetInsts) {
               if (RI->getFunction() != getAnchorScope())
@@ -4965,9 +4951,6 @@ struct AAValueSimplifyCallSiteReturned : AAValueSimplifyReturned {
 
   /// See AbstractAttribute::manifest(...).
   ChangeStatus manifest(Attributor &A) override {
-    if (auto *CI = dyn_cast<CallInst>(&getAssociatedValue()))
-      if (CI->isMustTailCall())
-        return ChangeStatus::UNCHANGED;
     return AAValueSimplifyImpl::manifest(A);
   }
 
@@ -7879,6 +7862,14 @@ ChangeStatus Attributor::run() {
       Use *U = It.first;
       Value *NewV = It.second;
       Value *OldV = U->get();
+
+      // Do not replace uses in returns if the value is a must-tail call we will
+      // not delete.
+      if (isa<ReturnInst>(U->getUser()))
+        if (auto *CI = dyn_cast<CallInst>(OldV->stripPointerCasts()))
+          if (CI->isMustTailCall() && !ToBeDeletedInsts.count(CI))
+            continue;
+
       LLVM_DEBUG(dbgs() << "Use " << *NewV << " in " << *U->getUser()
                         << " instead of " << *OldV << "\n");
       U->set(NewV);

diff  --git a/llvm/test/Transforms/Attributor/IPConstantProp/musttail-call.ll b/llvm/test/Transforms/Attributor/IPConstantProp/musttail-call.ll
index d5755da16555..d14c06760cb7 100644
--- a/llvm/test/Transforms/Attributor/IPConstantProp/musttail-call.ll
+++ b/llvm/test/Transforms/Attributor/IPConstantProp/musttail-call.ll
@@ -17,8 +17,7 @@ define i8* @start(i8 %v) {
 ; CHECK-NEXT:    [[C2:%.*]] = icmp eq i8 [[V]], 1
 ; CHECK-NEXT:    br i1 [[C2]], label [[C2_TRUE:%.*]], label [[C2_FALSE:%.*]]
 ; CHECK:       c2_true:
-; CHECK-NEXT:    [[CA1:%.*]] = musttail call i8* @no_side_effects(i8 undef)
-; CHECK-NEXT:    ret i8* [[CA1]]
+; CHECK-NEXT:    ret i8* null
 ; CHECK:       c2_false:
 ; CHECK-NEXT:    [[CA2:%.*]] = musttail call i8* @dont_zap_me(i8 undef)
 ; CHECK-NEXT:    ret i8* [[CA2]]
@@ -61,10 +60,6 @@ define internal i8* @side_effects(i8 %v) {
 }
 
 define internal i8* @no_side_effects(i8 %v) readonly nounwind {
-; CHECK-LABEL: define {{[^@]+}}@no_side_effects
-; CHECK-SAME: (i8 [[V:%.*]])
-; CHECK-NEXT:    ret i8* undef
-;
   ret i8* null
 }