[llvm] r347841 - [Inliner] Modify the merging of min-legal-vector-width attribute to better handle when the caller or callee don't have the attribute.

[Craig Topper via llvm-commits]

Author: ctopper
Date: Wed Nov 28 23:27:38 2018
New Revision: 347841

URL: http://llvm.org/viewvc/llvm-project?rev=347841&view=rev
Log:
[Inliner] Modify the merging of min-legal-vector-width attribute to better handle when the caller or callee don't have the attribute.

Lack of an attribute means that the function hasn't been checked for what vector width it requires. So if the caller or the callee doesn't have the attribute we should make sure the combined function after inlining does not have the attribute.

If the caller already doesn't have the attribute we can just avoid adding it. Otherwise if the callee doesn't have the attribute just remove the caller's attribute.

Modified:
    llvm/trunk/lib/IR/Attributes.cpp
    llvm/trunk/test/Transforms/Inline/inline-min-legal-vector-width.ll

Modified: llvm/trunk/lib/IR/Attributes.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/Attributes.cpp?rev=347841&r1=347840&r2=347841&view=diff
==============================================================================
--- llvm/trunk/lib/IR/Attributes.cpp (original)
+++ llvm/trunk/lib/IR/Attributes.cpp Wed Nov 28 23:27:38 2018
@@ -1685,28 +1685,32 @@ adjustCallerStackProbeSize(Function &Cal
 }
 
 /// If the inlined function defines a min legal vector width, then ensure
-/// the calling function has the same or larger min legal vector width. This
-/// function is called after the inlining decision has been made so we have to
-/// merge the attribute this way. Heuristics that would use
+/// the calling function has the same or larger min legal vector width. If the
+/// caller has the attribute, but the callee doesn't, we need to remove the
+/// attribute from the caller since we can't make any guarantees about the
+/// caller's requirements.
+/// This function is called after the inlining decision has been made so we have
+/// to merge the attribute this way. Heuristics that would use
 /// min-legal-vector-width to determine inline compatibility would need to be
 /// handled as part of inline cost analysis.
 static void
 adjustMinLegalVectorWidth(Function &Caller, const Function &Callee) {
-  if (Callee.hasFnAttribute("min-legal-vector-width")) {
-    uint64_t CalleeVectorWidth;
-    Callee.getFnAttribute("min-legal-vector-width")
-          .getValueAsString()
-          .getAsInteger(0, CalleeVectorWidth);
-    if (Caller.hasFnAttribute("min-legal-vector-width")) {
+  if (Caller.hasFnAttribute("min-legal-vector-width")) {
+    if (Callee.hasFnAttribute("min-legal-vector-width")) {
       uint64_t CallerVectorWidth;
       Caller.getFnAttribute("min-legal-vector-width")
             .getValueAsString()
             .getAsInteger(0, CallerVectorWidth);
-      if (CallerVectorWidth < CalleeVectorWidth) {
+      uint64_t CalleeVectorWidth;
+      Callee.getFnAttribute("min-legal-vector-width")
+            .getValueAsString()
+            .getAsInteger(0, CalleeVectorWidth);
+      if (CallerVectorWidth < CalleeVectorWidth)
         Caller.addFnAttr(Callee.getFnAttribute("min-legal-vector-width"));
-      }
     } else {
-      Caller.addFnAttr(Callee.getFnAttribute("min-legal-vector-width"));
+      // If the callee doesn't have the attribute then we don't know anything
+      // and must drop the attribute from the caller.
+      Caller.removeFnAttr("min-legal-vector-width");
     }
   }
 }

Modified: llvm/trunk/test/Transforms/Inline/inline-min-legal-vector-width.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/Inline/inline-min-legal-vector-width.ll?rev=347841&r1=347840&r2=347841&view=diff
==============================================================================
--- llvm/trunk/test/Transforms/Inline/inline-min-legal-vector-width.ll (original)
+++ llvm/trunk/test/Transforms/Inline/inline-min-legal-vector-width.ll Wed Nov 28 23:27:38 2018
@@ -8,6 +8,13 @@ define internal void @innerLarge() "min-
   ret void
 }
 
+define internal void @innerNoAttribute() {
+  ret void
+}
+
+; We should not add an attribute during inlining. No attribute means unknown.
+; Inlining doesn't change the fact that we don't know anything about this
+; function.
 define void @outerNoAttribute() {
   call void @innerLarge()
   ret void
@@ -23,7 +30,15 @@ define void @outerConflictingAttributeLa
   ret void
 }
 
-; CHECK: define void @outerNoAttribute() #0
+; We should remove the attribute after inlining since the callee's
+; vector width requirements are unknown.
+define void @outerAttribute() "min-legal-vector-width"="128" {
+  call void @innerNoAttribute()
+  ret void
+}
+
+; CHECK: define void @outerNoAttribute() {
 ; CHECK: define void @outerConflictingAttributeSmall() #0
 ; CHECK: define void @outerConflictingAttributeLarge() #0
+; CHECK: define void @outerAttribute() {
 ; CHECK: attributes #0 = { "min-legal-vector-width"="512" }