Correct 'target' default behavior on redecl, allow forward declaration.

Declarations without the attribute were disallowed because it would be
ambiguous which 'target' it was supposed to be on.  For example:

void ___attribute__((target("v1"))) foo();
void foo(); // Redecl of above, or fwd decl of below?
void ___attribute__((target("v2"))) foo();

However, a first declaration doesn't have that problem, and erroring
prevents it from working in cases where the forward declaration is
useful.

Additionally, a forward declaration of target==default wouldn't properly
cause multiversioning, so this patch fixes that.

The patch was not split since the 'default' fix would require
implementing the same check for that case, followed by undoing the same
change for the fwd-decl implementation.

Change-Id: I66f2c5bc2477bcd3f7544b9c16c83ece257077b0

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@347805 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/lib/AST/ASTContext.cpp b/lib/AST/ASTContext.cpp
index 1ac46f897c568871aa6bfdc9d11489e55954222a..464fa4bf4b598ad572151fedcae75ebe3dc6dc96 100644 (file)
--- a/lib/AST/ASTContext.cpp
+++ b/lib/AST/ASTContext.cpp
@@ -9921,10 +9921,10 @@ void ASTContext::forEachMultiversionedFunctionVersion(
     llvm::function_ref<void(FunctionDecl *)> Pred) const {
   assert(FD->isMultiVersion() && "Only valid for multiversioned functions");
   llvm::SmallDenseSet<const FunctionDecl*, 4> SeenDecls;
-  FD = FD->getCanonicalDecl();
+  FD = FD->getMostRecentDecl();
   for (auto *CurDecl :
        FD->getDeclContext()->getRedeclContext()->lookup(FD->getDeclName())) {
-    FunctionDecl *CurFD = CurDecl->getAsFunction()->getCanonicalDecl();
+    FunctionDecl *CurFD = CurDecl->getAsFunction()->getMostRecentDecl();
     if (CurFD && hasSameType(CurFD->getType(), FD->getType()) &&
         std::end(SeenDecls) == llvm::find(SeenDecls, CurFD)) {
       SeenDecls.insert(CurFD);

diff --git a/lib/CodeGen/CodeGenModule.cpp b/lib/CodeGen/CodeGenModule.cpp
index fa6cedd52f5cb985f3f678e056b79f06fc03190a..9ebd86a98193ff63832b6e24c85806debb67d3aa 100644 (file)
--- a/lib/CodeGen/CodeGenModule.cpp
+++ b/lib/CodeGen/CodeGenModule.cpp
@@ -1003,8 +1003,10 @@ void CodeGenModule::UpdateMultiVersionNames(GlobalDecl GD,
            "Other GD should now be a multiversioned function");
     // OtherFD is the version of this function that was mangled BEFORE
     // becoming a MultiVersion function.  It potentially needs to be updated.
-    const FunctionDecl *OtherFD =
-        OtherGD.getCanonicalDecl().getDecl()->getAsFunction();
+    const FunctionDecl *OtherFD = OtherGD.getCanonicalDecl()
+                                      .getDecl()
+                                      ->getAsFunction()
+                                      ->getMostRecentDecl();
     std::string OtherName = getMangledNameImpl(*this, OtherGD, OtherFD);
     // This is so that if the initial version was already the 'default'
     // version, we don't try to update it.

diff --git a/lib/Sema/SemaDecl.cpp b/lib/Sema/SemaDecl.cpp
index e34741608b2b3314d045f6f5079733ff4b26fc3b..8ccd9ae322b7eaee25983b37332889e0f19a9cca 100644 (file)
--- a/lib/Sema/SemaDecl.cpp
+++ b/lib/Sema/SemaDecl.cpp
@@ -9404,6 +9404,27 @@ static bool CheckMultiVersionValue(Sema &S, const FunctionDecl *FD) {
   return false;
 }
 
+static bool HasNonMultiVersionAttributes(const FunctionDecl *FD,
+                                         MultiVersionKind MVType) {
+  for (const Attr *A : FD->attrs()) {
+    switch (A->getKind()) {
+    case attr::CPUDispatch:
+    case attr::CPUSpecific:
+      if (MVType != MultiVersionKind::CPUDispatch &&
+          MVType != MultiVersionKind::CPUSpecific)
+        return true;
+      break;
+    case attr::Target:
+      if (MVType != MultiVersionKind::Target)
+        return true;
+      break;
+    default:
+      return true;
+    }
+  }
+  return false;
+}
+
 static bool CheckMultiVersionAdditionalRules(Sema &S, const FunctionDecl *OldFD,
                                              const FunctionDecl *NewFD,
                                              bool CausesMV,
@@ -9449,15 +9470,14 @@ static bool CheckMultiVersionAdditionalRules(Sema &S, const FunctionDecl *OldFD,
 
   // For now, disallow all other attributes.  These should be opt-in, but
   // an analysis of all of them is a future FIXME.
-  if (CausesMV && OldFD &&
-      std::distance(OldFD->attr_begin(), OldFD->attr_end()) != 1) {
+  if (CausesMV && OldFD && HasNonMultiVersionAttributes(OldFD, MVType)) {
     S.Diag(OldFD->getLocation(), diag::err_multiversion_no_other_attrs)
         << IsCPUSpecificCPUDispatchMVType;
     S.Diag(NewFD->getLocation(), diag::note_multiversioning_caused_here);
     return true;
   }
 
-  if (std::distance(NewFD->attr_begin(), NewFD->attr_end()) != 1)
+  if (HasNonMultiVersionAttributes(NewFD, MVType))
     return S.Diag(NewFD->getLocation(), diag::err_multiversion_no_other_attrs)
            << IsCPUSpecificCPUDispatchMVType;
 
@@ -9581,6 +9601,15 @@ static bool CheckMultiVersionFirstFunction(Sema &S, FunctionDecl *FD,
   return false;
 }
 
+static bool PreviousDeclsHaveMultiVersionAttribute(const FunctionDecl *FD) {
+  for (const Decl *D = FD->getPreviousDecl(); D; D = D->getPreviousDecl()) {
+    if (D->getAsFunction()->getMultiVersionKind() != MultiVersionKind::None)
+      return true;
+  }
+
+  return false;
+}
+
 static bool CheckTargetCausesMultiVersioning(
     Sema &S, FunctionDecl *OldFD, FunctionDecl *NewFD, const TargetAttr *NewTA,
     bool &Redeclaration, NamedDecl *&OldDecl, bool &MergeTypeWithPrevious,
@@ -9592,7 +9621,8 @@ static bool CheckTargetCausesMultiVersioning(
 
   // If the old decl is NOT MultiVersioned yet, and we don't cause that
   // to change, this is a simple redeclaration.
-  if (!OldTA || OldTA->getFeaturesStr() == NewTA->getFeaturesStr())
+  if (!NewTA->isDefaultVersion() &&
+      (!OldTA || OldTA->getFeaturesStr() == NewTA->getFeaturesStr()))
     return false;
 
   // Otherwise, this decl causes MultiVersioning.
@@ -9614,6 +9644,15 @@ static bool CheckTargetCausesMultiVersioning(
     return true;
   }
 
+  // If this is 'default', permit the forward declaration.
+  if (!OldFD->isMultiVersion() && !OldTA && NewTA->isDefaultVersion()) {
+    Redeclaration = true;
+    OldDecl = OldFD;
+    OldFD->setIsMultiVersion();
+    NewFD->setIsMultiVersion();
+    return false;
+  }
+
   if (CheckMultiVersionValue(S, OldFD)) {
     S.Diag(NewFD->getLocation(), diag::note_multiversioning_caused_here);
     NewFD->setInvalidDecl();
@@ -9632,7 +9671,10 @@ static bool CheckTargetCausesMultiVersioning(
 
   for (const auto *FD : OldFD->redecls()) {
     const auto *CurTA = FD->getAttr<TargetAttr>();
-    if (!CurTA || CurTA->isInherited()) {
+    // We allow forward declarations before ANY multiversioning attributes, but
+    // nothing after the fact.
+    if (PreviousDeclsHaveMultiVersionAttribute(FD) &&
+        (!CurTA || CurTA->isInherited())) {
       S.Diag(FD->getLocation(), diag::err_multiversion_required_in_redecl)
           << 0;
       S.Diag(NewFD->getLocation(), diag::note_multiversioning_caused_here);

diff --git a/test/CodeGen/attr-target-mv.c b/test/CodeGen/attr-target-mv.c
index f9fbbbd5f0bac0bb180d86448bb7d4a805b3e4d5..e22499e2928fde99425060b2d72e230df0b71a0b 100644 (file)
--- a/test/CodeGen/attr-target-mv.c
+++ b/test/CodeGen/attr-target-mv.c
@@ -35,10 +35,24 @@ void bar4() {
   foo_multi(1, 5.0);
 }
 
+int fwd_decl_default(void);
+int __attribute__((target("default"))) fwd_decl_default(void) { return 2; }
+
+int fwd_decl_avx(void);
+int __attribute__((target("avx"))) fwd_decl_avx(void) { return 2; }
+int __attribute__((target("default"))) fwd_decl_avx(void) { return 2; }
+
+void bar5() {
+  fwd_decl_default();
+  fwd_decl_avx();
+}
+
 // LINUX: @foo.ifunc = ifunc i32 (), i32 ()* ()* @foo.resolver
 // LINUX: @foo_inline.ifunc = ifunc i32 (), i32 ()* ()* @foo_inline.resolver
 // LINUX: @foo_decls.ifunc = ifunc void (), void ()* ()* @foo_decls.resolver
 // LINUX: @foo_multi.ifunc = ifunc void (i32, double), void (i32, double)* ()* @foo_multi.resolver
+// LINUX: @fwd_decl_default.ifunc = ifunc i32 (), i32 ()* ()* @fwd_decl_default.resolver
+// LINUX: @fwd_decl_avx.ifunc = ifunc i32 (), i32 ()* ()* @fwd_decl_avx.resolver
 
 // LINUX: define i32 @foo.sse4.2()
 // LINUX: ret i32 0
@@ -168,8 +182,45 @@ void bar4() {
 // WINDOWS: call void @foo_multi(i32 %0, double %1)
 // WINDOWS-NEXT: ret void
 
-// LINUX: declare i32 @foo.arch_sandybridge()
+// LINUX: define i32 @fwd_decl_default()
+// LINUX: ret i32 2
+// LINUX: define i32 @fwd_decl_avx.avx()
+// LINUX: ret i32 2
+// LINUX: define i32 @fwd_decl_avx()
+// LINUX: ret i32 2
 
+// WINDOWS: define dso_local i32 @fwd_decl_default()
+// WINDOWS: ret i32 2
+// WINDOWS: define dso_local i32 @fwd_decl_avx.avx()
+// WINDOWS: ret i32 2
+// WINDOWS: define dso_local i32 @fwd_decl_avx()
+// WINDOWS: ret i32 2
+
+// LINUX: define void @bar5()
+// LINUX: call i32 @fwd_decl_default.ifunc()
+// LINUX: call i32 @fwd_decl_avx.ifunc()
+
+// WINDOWS: define dso_local void @bar5()
+// WINDOWS: call i32 @fwd_decl_default.resolver()
+// WINDOWS: call i32 @fwd_decl_avx.resolver()
+
+// LINUX: define i32 ()* @fwd_decl_default.resolver() comdat
+// LINUX: call void @__cpu_indicator_init()
+// LINUX: ret i32 ()* @fwd_decl_default
+// LINUX: define i32 ()* @fwd_decl_avx.resolver() comdat
+// LINUX: call void @__cpu_indicator_init()
+// LINUX: ret i32 ()* @fwd_decl_avx.avx
+// LINUX: ret i32 ()* @fwd_decl_avx
+
+// WINDOWS: define dso_local i32 @fwd_decl_default.resolver() comdat
+// WINDOWS: call void @__cpu_indicator_init()
+// WINDOWS: call i32 @fwd_decl_default
+// WINDOWS: define dso_local i32 @fwd_decl_avx.resolver() comdat
+// WINDOWS: call void @__cpu_indicator_init()
+// WINDOWS: call i32 @fwd_decl_avx.avx
+// WINDOWS: call i32 @fwd_decl_avx
+
+// LINUX: declare i32 @foo.arch_sandybridge()
 // WINDOWS: declare dso_local i32 @foo.arch_sandybridge()
 
 // LINUX: define linkonce i32 @foo_inline.sse4.2()

diff --git a/test/Sema/attr-target-mv.c b/test/Sema/attr-target-mv.c
index 671adff5b0427d0e7a4ce00c6ffbfc3a10e7a1d3..664ade1c0fa5099f1413dabf284fe7deffaa29f6 100644 (file)
--- a/test/Sema/attr-target-mv.c
+++ b/test/Sema/attr-target-mv.c
@@ -65,10 +65,9 @@ int __attribute__((target("sse4.2,arch=sandybridge"))) mangle(void) { return 1;
 //expected-note@-2 {{previous declaration is here}}
 int __attribute__((target("arch=sandybridge,sse4.2")))  mangle(void) { return 2; }
 
+// allow this, since we want to treat the 1st one as fwd-decl of the sandybridge version.
 int prev_no_target(void);
 int __attribute__((target("arch=sandybridge")))  prev_no_target(void) { return 2; }
-// expected-error@-2 {{function declaration is missing 'target' attribute in a multiversioned function}}
-// expected-note@+1 {{function multiversioning caused by this declaration}}
 int __attribute__((target("arch=ivybridge")))  prev_no_target(void) { return 2; }
 
 int __attribute__((target("arch=sandybridge")))  prev_no_target2(void);