[CodeExtractor] Use subset of function attributes for extracted function.

In addition to target-dependent attributes, we can also preserve a
white-listed subset of target independent function attributes. The white-list
excludes problematic attributes, most prominently:

* attributes related to memory accesses, as alloca instructions
  could be moved in/out of the extracted block

* control-flow dependent attributes, like no_return or thunk, as the
  relerelevant instructions might or might not get extracted.

Thanks @efriedma and @aemerson for providing a set of attributes that cannot be
propagated.


Reviewers: efriedma, davidxl, davide, silvas

Reviewed By: efriedma

Differential Revision: https://reviews.llvm.org/D41334

llvm-svn: 321961

2 files changed, 159 insertions, 4 deletions

diff --git a/llvm/lib/Transforms/Utils/CodeExtractor.cpp b/llvm/lib/Transforms/Utils/CodeExtractor.cpp
index 7a404241cb1..08125962e0b 100644
--- a/llvm/lib/Transforms/Utils/CodeExtractor.cpp
+++ b/llvm/lib/Transforms/Utils/CodeExtractor.cpp
@@ -620,16 +620,86 @@ Function *CodeExtractor::constructFunction(const ValueSet &inputs,
   if (oldFunction->hasUWTable())
     newFunction->setHasUWTable();
 
-  // Inherit all of the target dependent attributes.
+  // Inherit all of the target dependent attributes and white-listed
+  // target independent attributes.
   //  (e.g. If the extracted region contains a call to an x86.sse
   //  instruction we need to make sure that the extracted region has the
   //  "target-features" attribute allowing it to be lowered.
   // FIXME: This should be changed to check to see if a specific
   //           attribute can not be inherited.
-  AttrBuilder AB(oldFunction->getAttributes().getFnAttributes());
-  for (const auto &Attr : AB.td_attrs())
-    newFunction->addFnAttr(Attr.first, Attr.second);
+  for (const auto &Attr : oldFunction->getAttributes().getFnAttributes()) {
+    if (Attr.isStringAttribute()) {
+      if (Attr.getKindAsString() == "thunk")
+        continue;
+    } else
+      switch (Attr.getKindAsEnum()) {
+      // Those attributes cannot be propagated safely. Explicitly list them
+      // here so we get a warning if new attributes are added. This list also
+      // includes non-function attributes.
+      case Attribute::Alignment:
+      case Attribute::AllocSize:
+      case Attribute::ArgMemOnly:
+      case Attribute::Builtin:
+      case Attribute::ByVal:
+      case Attribute::Convergent:
+      case Attribute::Dereferenceable:
+      case Attribute::DereferenceableOrNull:
+      case Attribute::InAlloca:
+      case Attribute::InReg:
+      case Attribute::InaccessibleMemOnly:
+      case Attribute::InaccessibleMemOrArgMemOnly:
+      case Attribute::JumpTable:
+      case Attribute::Naked:
+      case Attribute::Nest:
+      case Attribute::NoAlias:
+      case Attribute::NoBuiltin:
+      case Attribute::NoCapture:
+      case Attribute::NoReturn:
+      case Attribute::None:
+      case Attribute::NonNull:
+      case Attribute::ReadNone:
+      case Attribute::ReadOnly:
+      case Attribute::Returned:
+      case Attribute::ReturnsTwice:
+      case Attribute::SExt:
+      case Attribute::Speculatable:
+      case Attribute::StackAlignment:
+      case Attribute::StructRet:
+      case Attribute::SwiftError:
+      case Attribute::SwiftSelf:
+      case Attribute::WriteOnly:
+      case Attribute::ZExt:
+      case Attribute::EndAttrKinds:
+        continue;
+      // Those attributes should be safe to propagate to the extracted function.
+      case Attribute::AlwaysInline:
+      case Attribute::Cold:
+      case Attribute::NoRecurse:
+      case Attribute::InlineHint:
+      case Attribute::MinSize:
+      case Attribute::NoDuplicate:
+      case Attribute::NoImplicitFloat:
+      case Attribute::NoInline:
+      case Attribute::NonLazyBind:
+      case Attribute::NoRedZone:
+      case Attribute::NoUnwind:
+      case Attribute::OptimizeNone:
+      case Attribute::OptimizeForSize:
+      case Attribute::SafeStack:
+      case Attribute::SanitizeAddress:
+      case Attribute::SanitizeMemory:
+      case Attribute::SanitizeThread:
+      case Attribute::SanitizeHWAddress:
+      case Attribute::StackProtect:
+      case Attribute::StackProtectReq:
+      case Attribute::StackProtectStrong:
+      case Attribute::StrictFP:
+      case Attribute::UWTable:
+        break;
+      }
 
+    newFunction->addFnAttr(Attr);
+  }
   newFunction->getBasicBlockList().push_back(newRootNode);
 
   // Create an iterator to name all of the arguments we inserted.
diff --git a/llvm/test/Transforms/CodeExtractor/PartialInlineAttributes.ll b/llvm/test/Transforms/CodeExtractor/PartialInlineAttributes.ll
new file mode 100644
index 00000000000..40170846392
--- /dev/null
+++ b/llvm/test/Transforms/CodeExtractor/PartialInlineAttributes.ll
@@ -0,0 +1,85 @@
+; RUN: opt < %s -S -partial-inliner -skip-partial-inlining-cost-analysis=true | FileCheck %s
+
+
+define i32 @callee_most(i32 %v) unnamed_addr  #0 #1 {
+entry:
+  %cmp = icmp sgt i32 %v, 2000
+  br i1 %cmp, label %if.then, label %if.end
+
+if.then:
+  br label %if.then2
+
+if.then2:
+  %sub = sub i32 %v, 10
+  br label %if.end
+
+if.end:
+  %v2 = phi i32 [ %v, %entry ], [ %sub, %if.then2 ]
+  %add = add nsw i32 %v2, 200
+  ret i32 %add
+}
+
+define i32 @callee_noinline(i32 %v) optnone noinline {
+entry:
+  %cmp = icmp sgt i32 %v, 2000
+  br i1 %cmp, label %if.then, label %if.end
+
+if.then:
+  br label %if.then2
+
+if.then2:
+  %sub = sub i32 %v, 10
+  br label %if.end
+
+if.end:
+  %v2 = phi i32 [ %v, %entry ], [ %sub, %if.then2 ]
+  %add = add nsw i32 %v2, 200
+  ret i32 %add
+}
+
+define i32 @callee_writeonly(i32 %v) writeonly {
+entry:
+  %cmp = icmp sgt i32 %v, 2000
+  br i1 %cmp, label %if.then, label %if.end
+
+if.then:
+  br label %if.then2
+
+if.then2:
+  %sub = sub i32 %v, 10
+  br label %if.end
+
+if.end:
+  %v2 = phi i32 [ %v, %entry ], [ %sub, %if.then2 ]
+  %add = add nsw i32 %v2, 200
+  ret i32 %add
+}
+; CHECK-LABEL: @caller
+; CHECK: call void @callee_most.2_if.then(i32 %v
+; CHECK: call i32 @callee_noinline(i32 %v)
+; CHECK: call void @callee_writeonly.1_if.then(i32 %v
+define i32 @caller(i32 %v) {
+entry:
+  %c1 = call i32 @callee_most(i32 %v)
+  %c2 = call i32 @callee_noinline(i32 %v)
+  %c3 = call i32 @callee_writeonly(i32 %v)
+  ret i32 %c3
+}
+
+; CHECK: define internal void @callee_writeonly.1_if.then(i32 %v, i32* %sub.out) { 
+; CHECK: define internal void @callee_most.2_if.then(i32 %v, i32* %sub.out)  [[FN_ATTRS:#[0-9]+]]
+
+; attributes to preserve
+attributes #0 = {
+  inlinehint minsize noduplicate noimplicitfloat norecurse noredzone nounwind
+  nonlazybind optsize safestack sanitize_address sanitize_hwaddress sanitize_memory
+  sanitize_thread ssp sspreq sspstrong strictfp uwtable "foo"="bar"
+  "patchable-function"="prologue-short-redirect" "probe-stack"="_foo_guard" "stack-probe-size"="4096" }
+
+; CHECK: attributes [[FN_ATTRS]] = { inlinehint minsize noduplicate noimplicitfloat norecurse noredzone nounwind nonlazybind optsize safestack sanitize_address sanitize_hwaddress sanitize_memory sanitize_thread ssp sspreq sspstrong strictfp uwtable "foo"="bar" "patchable-function"="prologue-short-redirect" "probe-stack"="_foo_guard" "stack-probe-size"="4096" }
+
+; attributes to drop
+attributes #1 = {
+  alignstack=16 convergent inaccessiblememonly inaccessiblemem_or_argmemonly naked
+  noreturn readonly argmemonly returns_twice speculatable "thunk"
+}