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

[Nikita Popov via cfe-commits]

================
@@ -1381,21 +1405,58 @@ 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)
-          continue;
+        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, only remove from the
+          // existing AL if the region of the existing param is smaller than
+          // what we can propagate. AttributeList's merge API honours the
+          // already existing attribute value so we choose the "better"
+          // attribute by removing if the existing one is worse.
+          if (AL.getParamDereferenceableBytes(I) <
+              ValidExactParamAttrs[ArgNo].getDereferenceableBytes())
+            AL =
+                AL.removeParamAttribute(Context, I, Attribute::Dereferenceable);
+          if (AL.getParamDereferenceableOrNullBytes(I) <
+              ValidExactParamAttrs[ArgNo].getDereferenceableOrNullBytes())
+            AL =
+                AL.removeParamAttribute(Context, I, Attribute::Dereferenceable);
+          if (AL.getParamAlignment(I).valueOrOne() <
+              ValidExactParamAttrs[ArgNo].getAlignment().valueOrOne())
+            AL = AL.removeParamAttribute(Context, I, Attribute::Alignment);
+
+          auto ExistingRange = AL.getParamRange(I);
+          AL = AL.addParamAttributes(Context, I, ValidExactParamAttrs[ArgNo]);
+
+          // For range we use the intersection.
+          if (ExistingRange.has_value()) {
+            if (auto NewRange = ValidExactParamAttrs[ArgNo].getRange()) {
+              auto CombinedRange = ExistingRange->intersectWith(*NewRange);
+              AL = AL.removeParamAttribute(Context, I, Attribute::Range);
+              AL = AL.addRangeParamAttr(Context, I, CombinedRange);
+            }
+          }
+        } 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();
+        }
 
         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.
           continue;
----------------
nikic wrote:

Please apply this to all attributes, not just the underlying object ones. E.g. if you have a big dereferenceable and pass it to small byval, applying the big dereferenceable to the result would be incorrect. Alignment on byval also has very special semantics. Let's please not go there. At all.

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