[FunctionAttrs] Remove old "returned" argument deduction

[jdoerfert]

Summary

This removes the old FunctionAttrs "returned" argument deduction.

The code is obsolete as the Attributor deduction is more powerful.

The test cases that were affected by the change now run both passes.

No impact on the statistics (-stats) for LLVM-TS + Spec2006, this code is made obsolete by D59919.

[jdoerfert]

Minor changes

[reames]

Please post for review once attributor is actually enabled by default. Removing existing code before that point is inappropriate.

llvm/lib/Transforms/IPO/FunctionAttrs.cpp

 using namespace llvm;
 
 #define DEBUG_TYPE "functionattrs"
 
 STATISTIC(NumReadNone, "Number of functions marked readnone");
 STATISTIC(NumReadOnly, "Number of functions marked readonly");
 STATISTIC(NumWriteOnly, "Number of functions marked writeonly");
 STATISTIC(NumNoCapture, "Number of arguments marked nocapture");
-STATISTIC(NumReturned, "Number of arguments marked returned");
 STATISTIC(NumReadNoneArg, "Number of arguments marked readnone");
 STATISTIC(NumReadOnlyArg, "Number of arguments marked readonly");
 STATISTIC(NumNoAlias, "Number of function returns marked noalias");
 STATISTIC(NumNonNullReturn, "Number of function returns marked nonnull");
 STATISTIC(NumNoRecurse, "Number of functions marked as norecurse");
 STATISTIC(NumNoUnwind, "Number of functions marked as nounwind");
 
 // FIXME: This is disabled by default to avoid exposing security vulnerabilities
while (!Worklist.empty()) {
     default:
       return Attribute::None;
     }
   }
 
   return IsRead ? Attribute::ReadOnly : Attribute::ReadNone;
 }
 
-/// Deduce returned attributes for the SCC.
-static bool addArgumentReturnedAttrs(const SCCNodeSet &SCCNodes) {
-  bool Changed = false;
-
-  // Check each function in turn, determining if an argument is always returned.
-  for (Function *F : SCCNodes) {
-    // We can infer and propagate function attributes only when we know that the
-    // definition we'll get at link time is *exactly* the definition we see now.
-    // For more details, see GlobalValue::mayBeDerefined.
-    if (!F->hasExactDefinition())
-      continue;
-
-    if (F->getReturnType()->isVoidTy())
-      continue;
-
-    // There is nothing to do if an argument is already marked as 'returned'.
-    if (llvm::any_of(F->args(),
-                     [](const Argument &Arg) { return Arg.hasReturnedAttr(); }))
-      continue;
-
-    auto FindRetArg = [&]() -> Value * {
-      Value *RetArg = nullptr;
-      for (BasicBlock &BB : *F)
-        if (auto *Ret = dyn_cast<ReturnInst>(BB.getTerminator())) {
-          // Note that stripPointerCasts should look through functions with
-          // returned arguments.
-          Value *RetVal = Ret->getReturnValue()->stripPointerCasts();
-          if (!isa<Argument>(RetVal) || RetVal->getType() != F->getReturnType())
-            return nullptr;
-
-          if (!RetArg)
-            RetArg = RetVal;
-          else if (RetArg != RetVal)
-            return nullptr;
-        }
-
-      return RetArg;
-    };
-
-    if (Value *RetArg = FindRetArg()) {
-      auto *A = cast<Argument>(RetArg);
-      A->addAttr(Attribute::Returned);
-      ++NumReturned;
-      Changed = true;
-    }
-  }
-
-  return Changed;
-}
-
 /// If a callsite has arguments that are also arguments to the parent function,
 /// try to propagate attributes from the callsite's arguments to the parent's
 /// arguments. This may be important because inlining can cause information loss
 /// when attribute knowledge disappears with the inlined call.
 static bool addArgumentAttrsFromCallsites(Function &F) {
   if (!EnableNonnullArgPropagation)
     return false;
 

 static bool deriveAttrsInPostOrder(SCCNodeSet &SCCNodes, AARGetterT &&AARGetter,
                                    bool HasUnknownCall) {
   bool Changed = false;
 
   // Bail if the SCC only contains optnone functions.
   if (SCCNodes.empty())
     return Changed;
 
-  Changed |= addArgumentReturnedAttrs(SCCNodes);
   Changed |= addReadAttrs(SCCNodes, AARGetter);
   Changed |= addArgumentAttrs(SCCNodes);
 
   // If we have no external nodes participating in the SCC, we can deduce some
   // more precise attributes as well.
   if (!HasUnknownCall) {
     Changed |= addNoAliasAttrs(SCCNodes);
     Changed |= addNonNullAttrs(SCCNodes);

llvm/test/Transforms/FunctionAttrs/2009-01-02-LocalStores.ll

-; RUN: opt < %s -functionattrs -S | FileCheck %s
-; RUN: opt < %s -passes=function-attrs -S | FileCheck %s
+; RUN: opt < %s -attributor -functionattrs -S | FileCheck %s
+; RUN: opt < %s -passes='attributor,cgscc(function-attrs)' -S | FileCheck %s
 
 ; CHECK: define i32* @a(i32** nocapture readonly %p)
 define i32* @a(i32** %p) {
   %tmp = load i32*, i32** %p
   ret i32* %tmp
 }
 
 ; CHECK: define i32* @b(i32* %q)

llvm/test/Transforms/FunctionAttrs/arg_returned.ll

 
 ; TEST 1
 ;
 ; BOTH: define dso_local i32 @sink_r0(i32 returned %r) [[NoInlineNoRecurseNoUnwindReadnoneUwtable:#[0-9]]]
 ; BOTH: define dso_local i32 @scc_r1(i32 %a, i32 returned %r, i32 %b) [[NoInlineNoUnwindReadnoneUwtable:#[0-9]]]
 ; BOTH: define dso_local i32 @scc_r2(i32 %a, i32 %b, i32 returned %r) [[NoInlineNoUnwindReadnoneUwtable]]
 ; BOTH: define dso_local i32 @scc_rX(i32 %a, i32 %b, i32 %r) [[NoInlineNoUnwindReadnoneUwtable]]
 ;
-; FNATTR: define dso_local i32 @sink_r0(i32 returned %r) [[NoInlineNoRecurseNoUnwindReadnoneUwtable:#[0-9]]]
+; FNATTR: define dso_local i32 @sink_r0(i32 %r) [[NoInlineNoRecurseNoUnwindReadnoneUwtable:#[0-9]]]
 ; FNATTR: define dso_local i32 @scc_r1(i32 %a, i32 %r, i32 %b) [[NoInlineNoUnwindReadnoneUwtable:#[0-9]]]
 ; FNATTR: define dso_local i32 @scc_r2(i32 %a, i32 %b, i32 %r) [[NoInlineNoUnwindReadnoneUwtable]]
 ; FNATTR: define dso_local i32 @scc_rX(i32 %a, i32 %b, i32 %r) [[NoInlineNoUnwindReadnoneUwtable]]
 ;
 ; ATTRIBUTOR: define dso_local i32 @sink_r0(i32 returned %r) [[NoInlineNoRecurseNoUnwindReadnoneUwtable:#[0-9]]]
 ; ATTRIBUTOR: define dso_local i32 @scc_r1(i32 %a, i32 returned %r, i32 %b) [[NoInlineNoUnwindReadnoneUwtable:#[0-9]]]
 ; ATTRIBUTOR: define dso_local i32 @scc_r2(i32 %a, i32 %b, i32 returned %r) [[NoInlineNoUnwindReadnoneUwtable]]
 ; ATTRIBUTOR: define dso_local i32 @scc_rX(i32 %a, i32 %b, i32 %r) [[NoInlineNoUnwindReadnoneUwtable]]

 
 
 ; TEST 2
 ;
 ; BOTH: define dso_local double* @ptr_sink_r0(double* readnone returned %r) [[NoInlineNoRecurseNoUnwindReadnoneUwtable]]
 ; BOTH: define dso_local double* @ptr_scc_r1(double* %a, double* readnone returned %r, double* nocapture readnone %b) [[NoInlineNoUnwindReadnoneUwtable]]
 ; BOTH: define dso_local double* @ptr_scc_r2(double* readnone %a, double* readnone %b, double* readnone returned %r) [[NoInlineNoUnwindReadnoneUwtable]]
 ;
-; FNATTR: define dso_local double* @ptr_sink_r0(double* readnone returned %r) [[NoInlineNoRecurseNoUnwindReadnoneUwtable]]
+; FNATTR: define dso_local double* @ptr_sink_r0(double* readnone %r) [[NoInlineNoRecurseNoUnwindReadnoneUwtable]]
 ; FNATTR: define dso_local double* @ptr_scc_r1(double* %a, double* readnone %r, double* nocapture readnone %b) [[NoInlineNoUnwindReadnoneUwtable]]
 ; FNATTR: define dso_local double* @ptr_scc_r2(double* readnone %a, double* readnone %b, double* readnone %r) [[NoInlineNoUnwindReadnoneUwtable]]
 ;
 ; ATTRIBUTOR: define dso_local double* @ptr_sink_r0(double* returned %r) [[NoInlineNoRecurseNoUnwindReadnoneUwtable]]
 ; ATTRIBUTOR: define dso_local double* @ptr_scc_r1(double* %a, double* returned %r, double* %b) [[NoInlineNoUnwindReadnoneUwtable]]
 ; ATTRIBUTOR: define dso_local double* @ptr_scc_r2(double* %a, double* %b, double* returned %r) [[NoInlineNoUnwindReadnoneUwtable]]
 ;
 ; double* ptr_scc_r1(double* a, double* b, double* r);

 ;  int* calls_unknown_fn(int *r) {
 ;    unknown_fn(&calls_unknown_fn);
 ;    return r;
 ;  }
 ;
 ; BOTH: declare void @unknown_fn(i32* (i32*)*) [[NoInlineNoUnwindUwtable:#[0-9]*]]
 ;
 ; BOTH:       define dso_local i32* @calls_unknown_fn(i32* readnone returned %r) [[NoInlineNoUnwindUwtable]]
-; FNATTR:     define dso_local i32* @calls_unknown_fn(i32* readnone returned %r) [[NoInlineNoUnwindUwtable:#[0-9]*]]
+; FNATTR:     define dso_local i32* @calls_unknown_fn(i32* readnone %r) [[NoInlineNoUnwindUwtable:#[0-9]*]]
 ; ATTRIBUTOR: define dso_local i32* @calls_unknown_fn(i32* returned %r) [[NoInlineNoUnwindUwtable:#[0-9]*]]
 ;
 declare void @unknown_fn(i32* (i32*)*) #0
 
 define dso_local i32* @calls_unknown_fn(i32* %r) #0 {
   tail call void @unknown_fn(i32* (i32*)* nonnull @calls_unknown_fn)
   ret i32* %r
 }

 ;    return r;
 ;  }
 ;
 ; Verify the maybe-redefined function is not annotated:
 ; BOTH:  define linkonce_odr i32* @maybe_redefined_fn(i32* %r) [[NoInlineNoRecurseNoUnwindUwtable:#[0-9]*]]
 ; FIXME: We should not derive norecurse for potentially redefined functions!
 ;        define linkonce_odr i32* @maybe_redefined_fn(i32* %r) [[NoInlineNoUnwindUwtable]]
 ;
-; FNATTR:     define dso_local i32* @calls_maybe_redefined_fn(i32* returned %r) [[NoInlineNoRecurseNoUnwindUwtable:#[0-9]*]]
+; FNATTR:     define dso_local i32* @calls_maybe_redefined_fn(i32* %r) [[NoInlineNoRecurseNoUnwindUwtable:#[0-9]*]]
 ; ATTRIBUTOR: define dso_local i32* @calls_maybe_redefined_fn(i32* returned %r) [[NoInlineNoUnwindUwtable]]
 ; FIXME: We should not derive norecurse for potentially redefined functions!
 ;        define dso_local i32* @calls_maybe_redefined_fn(i32* returned %r) [[NoInlineNoUnwindUwtable]]
 ;
 define linkonce_odr i32* @maybe_redefined_fn(i32* %r) #0 {
 entry:
   ret i32* %r
 }

llvm/test/Transforms/FunctionAttrs/incompatible_fn_attrs.ll

-; RUN: opt -S -o - -functionattrs %s | FileCheck %s
-; RUN: opt -S -o - -passes=function-attrs %s | FileCheck %s
+; RUN: opt < %s -attributor -functionattrs -S | FileCheck %s
+; RUN: opt < %s -passes='attributor,cgscc(function-attrs)' -S | FileCheck %s
 
 ; Verify we remove argmemonly/inaccessiblememonly/inaccessiblemem_or_argmemonly
 ; function attributes when we derive readnone.
 
 ; Function Attrs: argmemonly
 define i32* @given_argmem_infer_readnone(i32* %p) #0 {
 ; CHECK: define i32* @given_argmem_infer_readnone(i32* readnone returned %p) #0 {
 entry:

llvm/test/Transforms/FunctionAttrs/nocapture.ll

-; RUN: opt < %s -functionattrs -S | FileCheck %s
-; RUN: opt < %s -passes=function-attrs -S | FileCheck %s
+; RUN: opt < %s -attributor -functionattrs -S | FileCheck %s
+; RUN: opt < %s -passes='attributor,cgscc(function-attrs)' -S | FileCheck %s
 
 @g = global i32* null   ; <i32**> [#uses=1]
 
 ; CHECK: define i32* @c1(i32* readnone returned %q)
 define i32* @c1(i32* %q) {
   ret i32* %q
 }
 

llvm/test/Transforms/FunctionAttrs/readattrs.ll

-; RUN: opt < %s -functionattrs -S | FileCheck %s
-; RUN: opt < %s -aa-pipeline=basic-aa -passes='cgscc(function-attrs)' -S | FileCheck %s
+; RUN: opt < %s -attributor -functionattrs -S | FileCheck %s
+; RUN: opt < %s -aa-pipeline=basic-aa -passes='attributor,cgscc(function-attrs)' -S | FileCheck %s
 @x = global i32 0
 
 declare void @test1_1(i8* %x1_1, i8* readonly %y1_1, ...)
 
 ; NOTE: readonly for %y1_2 would be OK here but not for the similar situation in test13.
 ;
 ; CHECK: define void @test1_2(i8* %x1_2, i8* readonly %y1_2, i8* %z1_2)
 define void @test1_2(i8* %x1_2, i8* %y1_2, i8* %z1_2) {