[clang] [llvm] [Inliner] Propagate more attributes to params when inlining (PR #91101)

Nikita Popov via llvm-commits llvm-commits at lists.llvm.org
Sat Oct 12 11:32:20 PDT 2024


================
@@ -1391,22 +1412,65 @@ static void AddParamAndFnBasicAttributes(const CallBase &CB,
 
       AttributeList AL = NewInnerCB->getAttributes();
       for (unsigned I = 0, E = InnerCB->arg_size(); I < E; ++I) {
-        // 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)
+        // It's unsound or requires special handling to propagate attributes to
+        // byval arguments. For reach even if CalledFunction doesn't e.g. write
+        // to the argument (readonly), the call to NewInnerCB may write to its
+        // by-value copy.
+        if (AL.hasParamAttr(I, Attribute::ByVal))
           continue;
 
-        if (AL.hasParamAttr(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.
+        // Don't both propagating attrs to constants.
+        if (match(NewInnerCB->getArgOperand(I),
+                  llvm::PatternMatch::m_ImmConstant()))
           continue;
 
-        unsigned ArgNo = Arg->getArgNo();
+        // 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 NewAL(Context);
+          for (auto Attr : ValidExactParamAttrs[ArgNo].attrs())
+            NewAL.addAttribute(Attr);
+          if (AL.getParamDereferenceableBytes(I) >
+              NewAL.getDereferenceableBytes())
+            NewAL.removeAttribute(Attribute::Dereferenceable);
+          if (AL.getParamDereferenceableOrNullBytes(I) >
+              NewAL.getDereferenceableOrNullBytes())
+            NewAL.removeAttribute(Attribute::Dereferenceable);
+          if (AL.getParamAlignment(I).valueOrOne() >
+              NewAL.getAlignment().valueOrOne())
+            NewAL.removeAttribute(Attribute::Alignment);
+
+          auto ExistingRange = AL.getParamRange(I);
----------------
nikic wrote:

Move ExistingRange below to the code using it. Though I think it would be cleaner if instead you just added the intersected attribute to the NewAL AttrBuilder instead of handling range separately. Then you also don't need the addRangeParamAttr API.

https://github.com/llvm/llvm-project/pull/91101


More information about the llvm-commits mailing list