Revert "[Inliner] Propagate more attributes to params when inlining (#91101)"

This reverts commit ae778ae.

Creates broken IR, see comments in #91101.

Author: aeubanks

clang/test/CodeGen/attr-counted-by-pr88931.cpp

@@ -13,7 +13,7 @@ void init(void * __attribute__((pass_dynamic_object_size(0))));
 // CHECK-LABEL: define dso_local void @_ZN3foo3barC1Ev(
 // CHECK-SAME: ptr noundef nonnull align 4 dereferenceable(1) [[THIS:%.*]]) unnamed_addr #[[ATTR0:[0-9]+]] align 2 {
 // CHECK-NEXT:  entry:
-// CHECK-NEXT:    tail call void @_Z4initPvU25pass_dynamic_object_size0(ptr noundef nonnull align 4 dereferenceable(1) [[THIS]], i64 noundef -1) #[[ATTR2:[0-9]+]]
+// CHECK-NEXT:    tail call void @_Z4initPvU25pass_dynamic_object_size0(ptr noundef nonnull [[THIS]], i64 noundef -1) #[[ATTR2:[0-9]+]]
 // CHECK-NEXT:    ret void
 //
 foo::bar::bar() {

clang/test/OpenMP/bug57757.cpp

@@ -39,7 +39,7 @@ void foo() {
 // CHECK-NEXT:    ]
 // CHECK:       .untied.jmp..i:
 // CHECK-NEXT:    store i32 1, ptr [[TMP2]], align 4, !tbaa [[TBAA16]], !alias.scope [[META13]], !noalias [[META17]]
-// CHECK-NEXT:    [[TMP4:%.*]] = tail call i32 @__kmpc_omp_task(ptr nonnull @[[GLOB1]], i32 [[TMP0]], ptr nonnull [[TMP1]]), !noalias [[META13]]
+// CHECK-NEXT:    [[TMP4:%.*]] = tail call i32 @__kmpc_omp_task(ptr nonnull @[[GLOB1]], i32 [[TMP0]], ptr [[TMP1]]), !noalias [[META13]]
 // CHECK-NEXT:    br label [[DOTOMP_OUTLINED__EXIT]]
 // CHECK:       .untied.next..i:
 // CHECK-NEXT:    [[TMP5:%.*]] = getelementptr inbounds nuw i8, ptr [[TMP1]], i64 40

llvm/include/llvm/IR/Attributes.h

@@ -947,9 +947,6 @@ class AttributeList {
   /// arg.
   uint64_t getParamDereferenceableOrNullBytes(unsigned ArgNo) const;
 
-  /// Get range (or std::nullopt if unknown) of an arg.
-  std::optional<ConstantRange> getParamRange(unsigned ArgNo) const;
-
   /// Get the disallowed floating-point classes of the return value.
   FPClassTest getRetNoFPClass() const;
 
@@ -1126,10 +1123,6 @@ class AttrBuilder {
   /// invalid if the Kind is not present in the builder.
   Attribute getAttribute(StringRef Kind) const;
 
-  /// Retrieve the range if the attribute exists (std::nullopt is returned
-  /// otherwise).
-  std::optional<ConstantRange> getRange() const;
-
   /// Return raw (possibly packed/encoded) value of integer attribute or
   /// std::nullopt if not set.
   std::optional<uint64_t> getRawIntAttr(Attribute::AttrKind Kind) const;

llvm/lib/IR/Attributes.cpp

@@ -1931,14 +1931,6 @@ AttributeList::getParamDereferenceableOrNullBytes(unsigned Index) const {
   return getParamAttrs(Index).getDereferenceableOrNullBytes();
 }
 
-std::optional<ConstantRange>
-AttributeList::getParamRange(unsigned ArgNo) const {
-  auto RangeAttr = getParamAttrs(ArgNo).getAttribute(Attribute::Range);
-  if (RangeAttr.isValid())
-    return RangeAttr.getRange();
-  return std::nullopt;
-}
-
 FPClassTest AttributeList::getRetNoFPClass() const {
   return getRetAttrs().getNoFPClass();
 }
@@ -2285,13 +2277,6 @@ Attribute AttrBuilder::getAttribute(StringRef A) const {
   return {};
 }
 
-std::optional<ConstantRange> AttrBuilder::getRange() const {
-  const Attribute RangeAttr = getAttribute(Attribute::Range);
-  if (RangeAttr.isValid())
-    return RangeAttr.getRange();
-  return std::nullopt;
-}
-
 bool AttrBuilder::contains(Attribute::AttrKind A) const {
   return getAttribute(A).isValid();
 }

llvm/lib/Transforms/Utils/InlineFunction.cpp

@@ -34,7 +34,6 @@
 #include "llvm/Analysis/VectorUtils.h"
 #include "llvm/IR/Argument.h"
 #include "llvm/IR/AttributeMask.h"
-#include "llvm/IR/Attributes.h"
 #include "llvm/IR/BasicBlock.h"
 #include "llvm/IR/CFG.h"
 #include "llvm/IR/Constant.h"
@@ -60,7 +59,6 @@
 #include "llvm/IR/MDBuilder.h"
 #include "llvm/IR/Metadata.h"
 #include "llvm/IR/Module.h"
-#include "llvm/IR/PatternMatch.h"
 #include "llvm/IR/ProfDataUtils.h"
 #include "llvm/IR/Type.h"
 #include "llvm/IR/User.h"
@@ -1360,36 +1358,18 @@ static void AddParamAndFnBasicAttributes(const CallBase &CB,
   auto &Context = CalledFunction->getContext();
 
   // Collect valid attributes for all params.
-  SmallVector<AttrBuilder> ValidObjParamAttrs, ValidExactParamAttrs;
+  SmallVector<AttrBuilder> ValidParamAttrs;
   bool HasAttrToPropagate = false;
 
-  // Attributes we can only propagate if the exact parameter is forwarded.
-  // We can propagate both poison generating and UB generating attributes
-  // without any extra checks. The only attribute that is tricky to propagate
-  // is `noundef` (skipped for now) as that can create new UB where previous
-  // behavior was just using a poison value.
-  static const Attribute::AttrKind ExactAttrsToPropagate[] = {
-      Attribute::Dereferenceable, Attribute::DereferenceableOrNull,
-      Attribute::NonNull, Attribute::Alignment, Attribute::Range};
-
   for (unsigned I = 0, E = CB.arg_size(); I < E; ++I) {
-    ValidObjParamAttrs.emplace_back(AttrBuilder{CB.getContext()});
-    ValidExactParamAttrs.emplace_back(AttrBuilder{CB.getContext()});
+    ValidParamAttrs.emplace_back(AttrBuilder{CB.getContext()});
     // Access attributes can be propagated to any param with the same underlying
     // object as the argument.
     if (CB.paramHasAttr(I, Attribute::ReadNone))
-      ValidObjParamAttrs.back().addAttribute(Attribute::ReadNone);
+      ValidParamAttrs.back().addAttribute(Attribute::ReadNone);
     if (CB.paramHasAttr(I, Attribute::ReadOnly))
-      ValidObjParamAttrs.back().addAttribute(Attribute::ReadOnly);
-
-    for (Attribute::AttrKind AK : ExactAttrsToPropagate) {
-      Attribute Attr = CB.getParamAttr(I, AK);
-      if (Attr.isValid())
-        ValidExactParamAttrs.back().addAttribute(Attr);
-    }
-
-    HasAttrToPropagate |= ValidObjParamAttrs.back().hasAttributes();
-    HasAttrToPropagate |= ValidExactParamAttrs.back().hasAttributes();
+      ValidParamAttrs.back().addAttribute(Attribute::ReadOnly);
+    HasAttrToPropagate |= ValidParamAttrs.back().hasAttributes();
   }
 
   // Won't be able to propagate anything.
@@ -1411,60 +1391,22 @@ static void AddParamAndFnBasicAttributes(const CallBase &CB,
 
       AttributeList AL = NewInnerCB->getAttributes();
       for (unsigned I = 0, E = InnerCB->arg_size(); I < E; ++I) {
-        // It's unsound or requires special handling to propagate
-        // attributes to byval arguments. Even if CalledFunction
-        // doesn't e.g. write to the argument (readonly), the call to
-        // NewInnerCB may write to its by-value copy.
-        if (NewInnerCB->paramHasAttr(I, Attribute::ByVal))
+        // Check if the underlying value for the parameter is an argument.
+        const Value *UnderlyingV =
+            getUnderlyingObject(InnerCB->getArgOperand(I));
+        const Argument *Arg = dyn_cast<Argument>(UnderlyingV);
+        if (!Arg)
           continue;
 
-        // Don't bother propagating attrs to constants.
-        if (match(NewInnerCB->getArgOperand(I),
-                  llvm::PatternMatch::m_ImmConstant()))
+        if (NewInnerCB->paramHasAttr(I, Attribute::ByVal))
+          // It's unsound to propagate memory attributes to byval arguments.
+          // Even if CalledFunction doesn't e.g. write to the argument,
+          // the call to NewInnerCB may write to its by-value copy.
           continue;
 
-        // Check if the underlying value for the parameter is an argument.
-        const Argument *Arg = dyn_cast<Argument>(InnerCB->getArgOperand(I));
-        unsigned ArgNo;
-        if (Arg) {
-          ArgNo = Arg->getArgNo();
-          // For dereferenceable, dereferenceable_or_null, align, etc...
-          // we don't want to propagate if the existing param has the same
-          // attribute with "better" constraints. So  remove from the
-          // new AL if the region of the existing param is larger than
-          // what we can propagate.
-          AttrBuilder NewAB{
-              Context, AttributeSet::get(Context, ValidExactParamAttrs[ArgNo])};
-          if (AL.getParamDereferenceableBytes(I) >
-              NewAB.getDereferenceableBytes())
-            NewAB.removeAttribute(Attribute::Dereferenceable);
-          if (AL.getParamDereferenceableOrNullBytes(I) >
-              NewAB.getDereferenceableOrNullBytes())
-            NewAB.removeAttribute(Attribute::DereferenceableOrNull);
-          if (AL.getParamAlignment(I).valueOrOne() >
-              NewAB.getAlignment().valueOrOne())
-            NewAB.removeAttribute(Attribute::Alignment);
-          if (auto ExistingRange = AL.getParamRange(I)) {
-            if (auto NewRange = NewAB.getRange()) {
-              ConstantRange CombinedRange =
-                  ExistingRange->intersectWith(*NewRange);
-              NewAB.removeAttribute(Attribute::Range);
-              NewAB.addRangeAttr(CombinedRange);
-            }
-          }
-          AL = AL.addParamAttributes(Context, I, NewAB);
-        } else {
-          // Check if the underlying value for the parameter is an argument.
-          const Value *UnderlyingV =
-              getUnderlyingObject(InnerCB->getArgOperand(I));
-          Arg = dyn_cast<Argument>(UnderlyingV);
-          if (!Arg)
-            continue;
-          ArgNo = Arg->getArgNo();
-        }
-
+        unsigned ArgNo = Arg->getArgNo();
         // If so, propagate its access attributes.
-        AL = AL.addParamAttributes(Context, I, ValidObjParamAttrs[ArgNo]);
+        AL = AL.addParamAttributes(Context, I, ValidParamAttrs[ArgNo]);
         // We can have conflicting attributes from the inner callsite and
         // to-be-inlined callsite. In that case, choose the most
         // restrictive.