[PATCH] D47474: Implement cpu_dispatch/cpu_specific Multiversioning
This revision was automatically updated to reflect the committed changes.
Closed by commit rC337552: Implement cpu_dispatch/cpu_specific Multiversioning
(authored by erichkeane, committed by ).
Repository:
rC Clang
https://reviews.llvm.org/D47474
Files:
include/clang/AST/Decl.h
include/clang/Basic/Attr.td
include/clang/Basic/AttrDocs.td
include/clang/Basic/DiagnosticGroups.td
include/clang/Basic/DiagnosticSemaKinds.td
include/clang/Basic/TargetInfo.h
include/clang/Basic/X86Target.def
lib/AST/Decl.cpp
lib/Basic/Targets/X86.cpp
lib/Basic/Targets/X86.h
lib/CodeGen/CGBuiltin.cpp
lib/CodeGen/CodeGenFunction.cpp
lib/CodeGen/CodeGenFunction.h
lib/CodeGen/CodeGenModule.cpp
lib/CodeGen/CodeGenModule.h
lib/Parse/ParseDecl.cpp
lib/Sema/AnalysisBasedWarnings.cpp
lib/Sema/Sema.cpp
lib/Sema/SemaDecl.cpp
lib/Sema/SemaDeclAttr.cpp
lib/Sema/SemaExpr.cpp
lib/Sema/SemaOverload.cpp
test/CodeGen/attr-cpuspecific.c
test/Misc/pragma-attribute-supported-attributes-list.test
test/Sema/attr-cpuspecific.c
test/SemaCXX/attr-cpuspecific.cpp
utils/TableGen/ClangAttrEmitter.cpp
Index: lib/AST/Decl.cpp
===
--- lib/AST/Decl.cpp
+++ lib/AST/Decl.cpp
@@ -2873,6 +2873,14 @@
return false;
}
+bool FunctionDecl::isCPUDispatchMultiVersion() const {
+ return isMultiVersion() && hasAttr();
+}
+
+bool FunctionDecl::isCPUSpecificMultiVersion() const {
+ return isMultiVersion() && hasAttr();
+}
+
void
FunctionDecl::setPreviousDeclaration(FunctionDecl *PrevDecl) {
redeclarable_base::setPreviousDecl(PrevDecl);
Index: lib/Sema/AnalysisBasedWarnings.cpp
===
--- lib/Sema/AnalysisBasedWarnings.cpp
+++ lib/Sema/AnalysisBasedWarnings.cpp
@@ -658,6 +658,11 @@
else
S.Diag(Loc, DiagID);
};
+
+ // cpu_dispatch functions permit empty function bodies for ICC compatibility.
+ if (D->getAsFunction() && D->getAsFunction()->isCPUDispatchMultiVersion())
+return;
+
// Either in a function body compound statement, or a function-try-block.
switch (CheckFallThrough(AC)) {
case UnknownFallThrough:
Index: lib/Sema/Sema.cpp
===
--- lib/Sema/Sema.cpp
+++ lib/Sema/Sema.cpp
@@ -1585,6 +1585,7 @@
}
bool Ambiguous = false;
+ bool IsMV = false;
if (Overloads) {
for (OverloadExpr::decls_iterator it = Overloads->decls_begin(),
@@ -1598,11 +1599,16 @@
if (const FunctionDecl *OverloadDecl
= dyn_cast((*it)->getUnderlyingDecl())) {
if (OverloadDecl->getMinRequiredArguments() == 0) {
- if (!ZeroArgCallReturnTy.isNull() && !Ambiguous) {
+ if (!ZeroArgCallReturnTy.isNull() && !Ambiguous &&
+ (!IsMV || !(OverloadDecl->isCPUDispatchMultiVersion() ||
+ OverloadDecl->isCPUSpecificMultiVersion( {
ZeroArgCallReturnTy = QualType();
Ambiguous = true;
- } else
+ } else {
ZeroArgCallReturnTy = OverloadDecl->getReturnType();
+IsMV = OverloadDecl->isCPUDispatchMultiVersion() ||
+ OverloadDecl->isCPUSpecificMultiVersion();
+ }
}
}
}
@@ -1683,7 +1689,7 @@
NamedDecl *Fn = (*It)->getUnderlyingDecl();
// Don't print overloads for non-default multiversioned functions.
if (const auto *FD = Fn->getAsFunction()) {
- if (FD->isMultiVersion() &&
+ if (FD->isMultiVersion() && FD->hasAttr() &&
!FD->getAttr()->isDefaultVersion())
continue;
}
@@ -1725,6 +1731,21 @@
!isa(E));
}
+static bool IsCPUDispatchCPUSpecificMultiVersion(const Expr *E) {
+ if (const auto *UO = dyn_cast(E))
+E = UO->getSubExpr();
+
+ if (const auto *ULE = dyn_cast(E)) {
+if (ULE->getNumDecls() == 0)
+ return false;
+
+const NamedDecl *ND = *ULE->decls_begin();
+if (const auto *FD = dyn_cast(ND))
+ return FD->isCPUDispatchMultiVersion() || FD->isCPUSpecificMultiVersion();
+ }
+ return false;
+}
+
bool Sema::tryToRecoverWithCall(ExprResult , const PartialDiagnostic ,
bool ForceComplain,
bool (*IsPlausibleResult)(QualType)) {
@@ -1741,12 +1762,13 @@
// so we can emit a fixit and carry on pretending that E was
// actually a CallExpr.
SourceLocation ParenInsertionLoc = getLocForEndOfToken(Range.getEnd());
-Diag(Loc, PD)
- << /*zero-arg*/ 1 << Range
- << (IsCallableWithAppend(E.get())
- ? FixItHint::CreateInsertion(ParenInsertionLoc, "()")
- : FixItHint());
-notePlausibleOverloads(*this, Loc, Overloads, IsPlausibleResult);
+bool IsMV = IsCPUDispatchCPUSpecificMultiVersion(E.get());
+Diag(Loc, PD) << /*zero-arg*/ 1 << IsMV << Range
+ << (IsCallableWithAppend(E.get())
+ ?
[PATCH] D47474: Implement cpu_dispatch/cpu_specific Multiversioning
aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. Aside from an `assert` than can be removed, this LGTM on the attribute side of things. Comment at: lib/CodeGen/CodeGenModule.cpp:2446 + const auto *FD = cast(GD.getDecl()); + assert(FD && "Not a FunctionDecl?"); + const auto *DD = FD->getAttr(); aaron.ballman wrote: > `cast<>` already asserts this. I don't think this is done -- the assert can be removed. https://reviews.llvm.org/D47474 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47474: Implement cpu_dispatch/cpu_specific Multiversioning
erichkeane added inline comments.
Comment at: include/clang/Basic/Attr.td:851
+ let Spellings = [Clang<"cpu_specific">];
+ let Args = [VariadicIdentifierArgument<"Cpus">];
+ let Subjects = SubjectList<[Function]>;
aaron.ballman wrote:
> erichkeane wrote:
> > aaron.ballman wrote:
> > > `Cpus` -> `CPUs` ?
> > So, I agree with you we should be consistent with spelling. However, in
> > this case, I'd prefer not changing this one. This is what the generated
> > code in Attrs.inc looks like if I change it:
> >
> > typedef IdentifierInfo ** cPUs_iterator;
> > cPUs_iterator cPUs_begin() const { return cPUs_; }
> > cPUs_iterator cPUs_end() const { return cPUs_ + cPUs_Size; }
> > unsigned cPUs_size() const { return cPUs_Size; }
> > llvm::iterator_range cPUs() const { return
> > llvm::make_range(cPUs_begin(), cPUs_end()); }
> >
> > I think having "Cpus" in 2 places is way better than having to spell it as
> > cPUs_begin. Your thoughts?
> Oh wow that is disgusting. Yes, please leave as `Cpus` here.
:-D Already Done!
https://reviews.llvm.org/D47474
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47474: Implement cpu_dispatch/cpu_specific Multiversioning
aaron.ballman added inline comments.
Comment at: include/clang/Basic/Attr.td:851
+ let Spellings = [Clang<"cpu_specific">];
+ let Args = [VariadicIdentifierArgument<"Cpus">];
+ let Subjects = SubjectList<[Function]>;
erichkeane wrote:
> aaron.ballman wrote:
> > `Cpus` -> `CPUs` ?
> So, I agree with you we should be consistent with spelling. However, in this
> case, I'd prefer not changing this one. This is what the generated code in
> Attrs.inc looks like if I change it:
>
> typedef IdentifierInfo ** cPUs_iterator;
> cPUs_iterator cPUs_begin() const { return cPUs_; }
> cPUs_iterator cPUs_end() const { return cPUs_ + cPUs_Size; }
> unsigned cPUs_size() const { return cPUs_Size; }
> llvm::iterator_range cPUs() const { return
> llvm::make_range(cPUs_begin(), cPUs_end()); }
>
> I think having "Cpus" in 2 places is way better than having to spell it as
> cPUs_begin. Your thoughts?
Oh wow that is disgusting. Yes, please leave as `Cpus` here.
https://reviews.llvm.org/D47474
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47474: Implement cpu_dispatch/cpu_specific Multiversioning
erichkeane added a comment.
Comment at: include/clang/AST/Decl.h:2212-2213
+ bool isCpuDispatchMultiVersion() const;
+ bool isCpuSpecificMultiVersion() const;
+
aaron.ballman wrote:
> aaron.ballman wrote:
> > Pedantic nit: CPU instead of Cpu?
> Thoughts on `isCPUDispatchMultiVersion()` instead of
> `isCpuDispatchMultiVersion()`?
Well, I can definitely switch it back if CPU is your preference. Looking at
it, we're probably about 75/25 in our codebase preferring CPU, so I prefer it
as well.
Comment at: include/clang/Basic/Attr.td:851
+ let Spellings = [Clang<"cpu_specific">];
+ let Args = [VariadicIdentifierArgument<"Cpus">];
+ let Subjects = SubjectList<[Function]>;
aaron.ballman wrote:
> `Cpus` -> `CPUs` ?
So, I agree with you we should be consistent with spelling. However, in this
case, I'd prefer not changing this one. This is what the generated code in
Attrs.inc looks like if I change it:
typedef IdentifierInfo ** cPUs_iterator;
cPUs_iterator cPUs_begin() const { return cPUs_; }
cPUs_iterator cPUs_end() const { return cPUs_ + cPUs_Size; }
unsigned cPUs_size() const { return cPUs_Size; }
llvm::iterator_range cPUs() const { return
llvm::make_range(cPUs_begin(), cPUs_end()); }
I think having "Cpus" in 2 places is way better than having to spell it as
cPUs_begin. Your thoughts?
Comment at: lib/Sema/SemaOverload.cpp:9017
+
+return (*FirstDiff.first)->getName() < (*FirstDiff.second)->getName();
+ }
aaron.ballman wrote:
> If there's no mismatch, doesn't this wind up dereferencing the end iterator
> of the range?
Yikes, you're right! However, that would require 2 declarations to have
identical cpu_specific lists, which is against the rules previously enforced.
I've put an assert in here to ensure that never happens.
https://reviews.llvm.org/D47474
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47474: Implement cpu_dispatch/cpu_specific Multiversioning
erichkeane updated this revision to Diff 156068.
erichkeane marked 13 inline comments as done.
erichkeane added a comment.
All of @aaron.ballman s comments have been dealt with except for Cpus->CPUs in
the Attr.td file for explained reasons. Still negotiable :)
https://reviews.llvm.org/D47474
Files:
include/clang/AST/Decl.h
include/clang/Basic/Attr.td
include/clang/Basic/AttrDocs.td
include/clang/Basic/DiagnosticGroups.td
include/clang/Basic/DiagnosticSemaKinds.td
include/clang/Basic/TargetInfo.h
include/clang/Basic/X86Target.def
lib/AST/Decl.cpp
lib/Basic/Targets/X86.cpp
lib/Basic/Targets/X86.h
lib/CodeGen/CGBuiltin.cpp
lib/CodeGen/CodeGenFunction.cpp
lib/CodeGen/CodeGenFunction.h
lib/CodeGen/CodeGenModule.cpp
lib/CodeGen/CodeGenModule.h
lib/Parse/ParseDecl.cpp
lib/Sema/AnalysisBasedWarnings.cpp
lib/Sema/Sema.cpp
lib/Sema/SemaDecl.cpp
lib/Sema/SemaDeclAttr.cpp
lib/Sema/SemaExpr.cpp
lib/Sema/SemaOverload.cpp
test/CodeGen/attr-cpuspecific.c
test/Misc/pragma-attribute-supported-attributes-list.test
test/Sema/attr-cpuspecific.c
test/SemaCXX/attr-cpuspecific.cpp
utils/TableGen/ClangAttrEmitter.cpp
Index: utils/TableGen/ClangAttrEmitter.cpp
===
--- utils/TableGen/ClangAttrEmitter.cpp
+++ utils/TableGen/ClangAttrEmitter.cpp
@@ -1173,6 +1173,13 @@
}
};
+ class VariadicIdentifierArgument : public VariadicArgument {
+ public:
+VariadicIdentifierArgument(const Record , StringRef Attr)
+ : VariadicArgument(Arg, Attr, "IdentifierInfo *")
+{}
+ };
+
class VariadicStringArgument : public VariadicArgument {
public:
VariadicStringArgument(const Record , StringRef Attr)
@@ -1278,6 +1285,8 @@
Ptr = llvm::make_unique(Arg, Attr);
else if (ArgName == "ParamIdxArgument")
Ptr = llvm::make_unique(Arg, Attr, "ParamIdx");
+ else if (ArgName == "VariadicIdentifierArgument")
+Ptr = llvm::make_unique(Arg, Attr);
else if (ArgName == "VersionArgument")
Ptr = llvm::make_unique(Arg, Attr);
@@ -2106,6 +2115,34 @@
.Default(false);
}
+static bool isVariadicIdentifierArgument(Record *Arg) {
+ return !Arg->getSuperClasses().empty() &&
+ llvm::StringSwitch(
+ Arg->getSuperClasses().back().first->getName())
+ .Case("VariadicIdentifierArgument", true)
+ .Default(false);
+}
+
+static void emitClangAttrVariadicIdentifierArgList(RecordKeeper ,
+ raw_ostream ) {
+ OS << "#if defined(CLANG_ATTR_VARIADIC_IDENTIFIER_ARG_LIST)\n";
+ std::vector Attrs = Records.getAllDerivedDefinitions("Attr");
+ for (const auto *A : Attrs) {
+// Determine whether the first argument is a variadic identifier.
+std::vector Args = A->getValueAsListOfDefs("Args");
+if (Args.empty() || !isVariadicIdentifierArgument(Args[0]))
+ continue;
+
+// All these spellings take an identifier argument.
+forEachUniqueSpelling(*A, [&](const FlattenedSpelling ) {
+ OS << ".Case(\"" << S.name() << "\", "
+ << "true"
+ << ")\n";
+});
+ }
+ OS << "#endif // CLANG_ATTR_VARIADIC_IDENTIFIER_ARG_LIST\n\n";
+}
+
// Emits the first-argument-is-identifier property for attributes.
static void emitClangAttrIdentifierArgList(RecordKeeper , raw_ostream ) {
OS << "#if defined(CLANG_ATTR_IDENTIFIER_ARG_LIST)\n";
@@ -3697,6 +3734,7 @@
emitSourceFileHeader("Parser-related llvm::StringSwitch cases", OS);
emitClangAttrArgContextList(Records, OS);
emitClangAttrIdentifierArgList(Records, OS);
+ emitClangAttrVariadicIdentifierArgList(Records, OS);
emitClangAttrTypeArgList(Records, OS);
emitClangAttrLateParsedList(Records, OS);
}
Index: test/SemaCXX/attr-cpuspecific.cpp
===
--- /dev/null
+++ test/SemaCXX/attr-cpuspecific.cpp
@@ -0,0 +1,111 @@
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -fsyntax-only -verify -fexceptions -fcxx-exceptions %s -std=c++14
+
+// expected-error@+1{{invalid option 'invalid' for cpu_dispatch}}
+void __attribute__((cpu_dispatch(atom, invalid))) invalid_cpu();
+
+void __attribute__((cpu_specific(atom))) no_default(void);
+void __attribute__((cpu_specific(sandybridge))) no_default(void);
+
+struct MVReference {
+ int __attribute__((cpu_specific(sandybridge))) bar(void);
+ int __attribute__((cpu_specific(ivybridge))) bar(void);
+ int __attribute__((cpu_specific(sandybridge))) foo(void);
+};
+
+void use1(void){
+ // OK, will fail in the linker, unless another TU provides the cpu_dispatch.
+ no_default();
+
+ // expected-error@+1 {{call to non-static member function without an object argument}}
+ +MVReference::bar;
+ // expected-error@+1 {{call to non-static member function without an object argument}}
+ +MVReference::foo;
+ // expected-error@+1 {{reference to multiversioned function could not be resolved; did you mean to call it?}}
+ ::bar;
+ //
[PATCH] D47474: Implement cpu_dispatch/cpu_specific Multiversioning
aaron.ballman added inline comments.
Comment at: include/clang/AST/Decl.h:2212-2213
+ bool isCpuDispatchMultiVersion() const;
+ bool isCpuSpecificMultiVersion() const;
+
aaron.ballman wrote:
> Pedantic nit: CPU instead of Cpu?
Thoughts on `isCPUDispatchMultiVersion()` instead of
`isCpuDispatchMultiVersion()`?
Comment at: include/clang/Basic/Attr.td:851
+ let Spellings = [Clang<"cpu_specific">];
+ let Args = [VariadicIdentifierArgument<"Cpus">];
+ let Subjects = SubjectList<[Function]>;
`Cpus` -> `CPUs` ?
Comment at: include/clang/Basic/Attr.td:865
+ let Spellings = [Clang<"cpu_dispatch">];
+ let Args = [VariadicIdentifierArgument<"Cpus">];
+ let Subjects = SubjectList<[Function]>;
`Cpus` -> `CPUs` ?
Comment at: include/clang/Basic/Attr.td:851
+ let Spellings = [GCC<"cpu_specific">];
+ let Args = [VariadicIdentifierArgument<"Cpus">];
+ let Subjects = SubjectList<[Function]>;
erichkeane wrote:
> aaron.ballman wrote:
> > erichkeane wrote:
> > > aaron.ballman wrote:
> > > > Be sure to add a test for using this attribute with the C++ spelling,
> > > > as I'm not certain how well we would parse something like
> > > > `[[gnu::cpu_specific(ivybridge)]]` currently (it may just work,
> > > > however).
> > > >
> > > > Also, why an identifier instead of a string literal?
> > > I'll add it, presumably as 'clang::cpu_specific'. The decision for
> > > string-literal vs identifier was made quite a few years before I was here
> > > sadly. I believe the EDG FE doesn't make identifiers any more difficult
> > > so the implementer here chose to make it that way.
> > >
> > > In this case, I'd very much like to keep it with the same implementation
> > > as ICC, simply because users of this are already familiar with it in this
> > > form.
> > > In this case, I'd very much like to keep it with the same implementation
> > > as ICC, simply because users of this are already familiar with it in this
> > > form.
> >
> > The compatibility with ICC is important for the GNU-style attribute, but
> > for the C++ spelling this is novel territory where there is no
> > compatibility story. Another approach to consider is whether to accept
> > identifiers or string literals depending on the spelling, but that might
> > not be worth it.
> I'd like to think about that... I could forsee accepting BOTH forms, simply
> because it would slightly simplify the conversion from an attribute-target-mv
> situation, though I'm not sure it is important enough to do.
>
>
I'm okay with the current approach that uses identifiers only. We can relax the
rule to allow string literals if it turns out there is user demand for such a
thing.
Comment at: include/clang/Basic/AttrDocs.td:247
+It is also possible to specify a CPU name of ``generic``, which is the
+condition-less implementation, which will be resolved if the executing
processor
+doesn't satisfy the features required in the CPU name. The behavior of a
program
I'd drop the bit about "which is the condition-less implementation". It reads a
bit oddly to begin with and doesn't really add much to the explanation.
Comment at: lib/CodeGen/CodeGenModule.cpp:866
+ StringRef Name) {
+ const auto = CGM.getTarget();
+ return (Twine('.') + Twine(Target.CPUSpecificManglingCharacter(Name))).str();
Don't use `auto` here.
Comment at: lib/CodeGen/CodeGenModule.cpp:2446
+ const auto *FD = cast(GD.getDecl());
+ assert(FD && "Not a FunctionDecl?");
+ const auto *DD = FD->getAttr();
`cast<>` already asserts this.
Comment at: lib/CodeGen/CodeGenModule.cpp:2457
+ false);
+ llvm::Function *ResolverFunc = cast(
+ GetOrCreateLLVMFunction(ResolverName, ResolverType, GlobalDecl{},
Can use `auto *` here.
Comment at: lib/CodeGen/CodeGenModule.cpp:2464
+ const TargetInfo = getTarget();
+ for (IdentifierInfo *II : DD->cpus()) {
+// Get the name of the target function so we can look it up/create it.
`const IdentifierInfo *`
Comment at: lib/Sema/Sema.cpp:1733
+static bool IsCPUDispatchCPUSpecificMultiVersion(Expr *E) {
+ if (const auto *UO = dyn_cast(E))
`const Expr *`?
Comment at: lib/Sema/SemaDecl.cpp:9587
+ } else if (NewMVType == MultiVersioning::CPUSpecific && CurCPUSpec) {
+
+if (CurCPUSpec->cpus_size() == NewCPUSpec->cpus_size() &&
Spurious newline? Also, `else` after a return.
Comment at: lib/Sema/SemaDecl.cpp:9614
+ }
+ // if The two decls aren't the same MVType, there is no possible error
+ // condition.
s/if The/If the
Comment at:
[PATCH] D47474: Implement cpu_dispatch/cpu_specific Multiversioning
erichkeane added a comment. Bump! Would love to have someone take a look! https://reviews.llvm.org/D47474 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47474: Implement cpu_dispatch/cpu_specific Multiversioning
erichkeane updated this revision to Diff 154610.
erichkeane marked an inline comment as done.
erichkeane added a comment.
@lebedev.ri 's comments, plus a rebase.
https://reviews.llvm.org/D47474
Files:
include/clang/AST/Decl.h
include/clang/Basic/Attr.td
include/clang/Basic/AttrDocs.td
include/clang/Basic/DiagnosticGroups.td
include/clang/Basic/DiagnosticSemaKinds.td
include/clang/Basic/TargetInfo.h
include/clang/Basic/X86Target.def
lib/AST/Decl.cpp
lib/Basic/Targets/X86.cpp
lib/Basic/Targets/X86.h
lib/CodeGen/CGBuiltin.cpp
lib/CodeGen/CodeGenFunction.cpp
lib/CodeGen/CodeGenFunction.h
lib/CodeGen/CodeGenModule.cpp
lib/CodeGen/CodeGenModule.h
lib/Parse/ParseDecl.cpp
lib/Sema/AnalysisBasedWarnings.cpp
lib/Sema/Sema.cpp
lib/Sema/SemaDecl.cpp
lib/Sema/SemaDeclAttr.cpp
lib/Sema/SemaExpr.cpp
lib/Sema/SemaOverload.cpp
test/CodeGen/attr-cpuspecific.c
test/Misc/pragma-attribute-supported-attributes-list.test
test/Sema/attr-cpuspecific.c
test/SemaCXX/attr-cpuspecific.cpp
utils/TableGen/ClangAttrEmitter.cpp
Index: utils/TableGen/ClangAttrEmitter.cpp
===
--- utils/TableGen/ClangAttrEmitter.cpp
+++ utils/TableGen/ClangAttrEmitter.cpp
@@ -1173,6 +1173,13 @@
}
};
+ class VariadicIdentifierArgument : public VariadicArgument {
+ public:
+VariadicIdentifierArgument(const Record , StringRef Attr)
+ : VariadicArgument(Arg, Attr, "IdentifierInfo *")
+{}
+ };
+
class VariadicStringArgument : public VariadicArgument {
public:
VariadicStringArgument(const Record , StringRef Attr)
@@ -1278,6 +1285,8 @@
Ptr = llvm::make_unique(Arg, Attr);
else if (ArgName == "ParamIdxArgument")
Ptr = llvm::make_unique(Arg, Attr, "ParamIdx");
+ else if (ArgName == "VariadicIdentifierArgument")
+Ptr = llvm::make_unique(Arg, Attr);
else if (ArgName == "VersionArgument")
Ptr = llvm::make_unique(Arg, Attr);
@@ -2106,6 +2115,34 @@
.Default(false);
}
+static bool isVariadicIdentifierArgument(Record *Arg) {
+ return !Arg->getSuperClasses().empty() &&
+ llvm::StringSwitch(
+ Arg->getSuperClasses().back().first->getName())
+ .Case("VariadicIdentifierArgument", true)
+ .Default(false);
+}
+
+static void emitClangAttrVariadicIdentifierArgList(RecordKeeper ,
+ raw_ostream ) {
+ OS << "#if defined(CLANG_ATTR_VARIADIC_IDENTIFIER_ARG_LIST)\n";
+ std::vector Attrs = Records.getAllDerivedDefinitions("Attr");
+ for (const auto *A : Attrs) {
+// Determine whether the first argument is a variadic identifier.
+std::vector Args = A->getValueAsListOfDefs("Args");
+if (Args.empty() || !isVariadicIdentifierArgument(Args[0]))
+ continue;
+
+// All these spellings take an identifier argument.
+forEachUniqueSpelling(*A, [&](const FlattenedSpelling ) {
+ OS << ".Case(\"" << S.name() << "\", "
+ << "true"
+ << ")\n";
+});
+ }
+ OS << "#endif // CLANG_ATTR_VARIADIC_IDENTIFIER_ARG_LIST\n\n";
+}
+
// Emits the first-argument-is-identifier property for attributes.
static void emitClangAttrIdentifierArgList(RecordKeeper , raw_ostream ) {
OS << "#if defined(CLANG_ATTR_IDENTIFIER_ARG_LIST)\n";
@@ -3696,6 +3733,7 @@
emitSourceFileHeader("Parser-related llvm::StringSwitch cases", OS);
emitClangAttrArgContextList(Records, OS);
emitClangAttrIdentifierArgList(Records, OS);
+ emitClangAttrVariadicIdentifierArgList(Records, OS);
emitClangAttrTypeArgList(Records, OS);
emitClangAttrLateParsedList(Records, OS);
}
Index: test/SemaCXX/attr-cpuspecific.cpp
===
--- /dev/null
+++ test/SemaCXX/attr-cpuspecific.cpp
@@ -0,0 +1,111 @@
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -fsyntax-only -verify -fexceptions -fcxx-exceptions %s -std=c++14
+
+// expected-error@+1{{invalid option 'invalid' for cpu_dispatch}}
+void __attribute__((cpu_dispatch(atom, invalid))) invalid_cpu();
+
+void __attribute__((cpu_specific(atom))) no_default(void);
+void __attribute__((cpu_specific(sandybridge))) no_default(void);
+
+struct MVReference {
+ int __attribute__((cpu_specific(sandybridge))) bar(void);
+ int __attribute__((cpu_specific(ivybridge))) bar(void);
+ int __attribute__((cpu_specific(sandybridge))) foo(void);
+};
+
+void use1(void){
+ // OK, will fail in the linker, unless another TU provides the cpu_dispatch.
+ no_default();
+
+ // expected-error@+1 {{call to non-static member function without an object argument}}
+ +MVReference::bar;
+ // expected-error@+1 {{call to non-static member function without an object argument}}
+ +MVReference::foo;
+ // expected-error@+1 {{reference to multiversioned function could not be resolved; did you mean to call it?}}
+ ::bar;
+ // expected-error@+1 {{reference to multiversioned function could not be resolved; did you mean to call it?}}
+
[PATCH] D47474: Implement cpu_dispatch/cpu_specific Multiversioning
erichkeane marked 4 inline comments as done.
erichkeane added a comment.
In https://reviews.llvm.org/D47474#1154858, @lebedev.ri wrote:
> Some drive-by nits.
> General observation: this is kinda large.
> I would //personally// split split off the codegen part into a second patch.
Thanks for the review! I definitely agree this is a sizable patch, I'm not
sure the non-codegen part has any value without the codegen so I'm not sure
that splitting it would provide value. I can definitely do so if reviewers in
general believe this is the right tack.
Comment at: include/clang/Basic/X86Target.def:288
+
+// FIXME: When commented out features are supported in LLVM, enable them here.
+CPU_SPECIFIC("generic", 'A', "")
lebedev.ri wrote:
> This is going to go stale.
> It already does not have znver1, btver2.
> Surely this info already exists elsewhere in llvm?
> Can it be deduplicated somehow?
You'll note that this doesn't have any non-intel names :) This is non-Intel
processor friendly version of the ICC implementation of the same feature.
Because of this, I don't have a good feature list for those.
I'd searched in a few places for an alternate source for these names/feature
lists. I was unable to find anywhere else that needs/has similar information,
though any alternate sources are greatly appreciated.
I'll note that the name and mangling-name aren't going to be able to be
removed, though perhaps there is a source of these features somewhere?
Comment at: lib/CodeGen/CodeGenModule.cpp:868
+ const auto = CGM.getTarget();
+ return std::string(".") + Target.CPUSpecificManglingCharacter(Name);
+}
lebedev.ri wrote:
> I think
> ```
> return Twine(".", Target.CPUSpecificManglingCharacter(Name)).str();
> ```
A twine of char + a char is actually pretty challenging... I rewrote it without
the intermediate string though. I am generally unconcerned, since 2 chars is
very much in the 'short string optimization' range of all implementations.
https://reviews.llvm.org/D47474
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47474: Implement cpu_dispatch/cpu_specific Multiversioning
lebedev.ri added a comment.
Some drive-by nits.
General observation: this is kinda large.
I would //personally// split split off the codegen part into a second patch.
Comment at: include/clang/AST/Decl.h:2212-2213
+ bool isCPUDispatchMultiVersion() const;
+ bool isCPUSpecificMultiVersion() const;
+
Everything else has comments, and i'm not sure it is obvious what those mean
without knowing the context.
Comment at: include/clang/Basic/AttrDocs.td:223
+A dispatching (or resolving) function can be declared anywhere in your
+compilation with ``cpu_dispatch``. This attribute takes one or more CPU names
+as a parameter (like ``cpu_specific``). Functions marked with ``cpu_dispatch``
s/compilation/source code/?
Comment at: include/clang/Basic/X86Target.def:288
+
+// FIXME: When commented out features are supported in LLVM, enable them here.
+CPU_SPECIFIC("generic", 'A', "")
This is going to go stale.
It already does not have znver1, btver2.
Surely this info already exists elsewhere in llvm?
Can it be deduplicated somehow?
Comment at: lib/CodeGen/CodeGenFunction.cpp:2421
+ // Main function's basic block.
+ llvm::BasicBlock *CurBlock = createBasicBlock("entry", Resolver);
+ Builder.SetInsertPoint(CurBlock);
Those are arbitrary names, right?
Maybe somehow convey that this is CPUDispatchMultiVersionResolver logic in the
names?
Comment at: lib/CodeGen/CodeGenModule.cpp:868
+ const auto = CGM.getTarget();
+ return std::string(".") + Target.CPUSpecificManglingCharacter(Name);
+}
I think
```
return Twine(".", Target.CPUSpecificManglingCharacter(Name)).str();
```
https://reviews.llvm.org/D47474
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47474: Implement cpu_dispatch/cpu_specific Multiversioning
erichkeane added a comment. Bump! https://reviews.llvm.org/D47474 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47474: Implement cpu_dispatch/cpu_specific Multiversioning
erichkeane added a comment. Just a quick bump, I know most of you are still recovering from the Switzerland trip, but hopefully the others will have some time. https://reviews.llvm.org/D47474 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47474: Implement cpu_dispatch/cpu_specific Multiversioning
erichkeane updated this revision to Diff 149276.
erichkeane added a comment.
Shamelessly stealing @aaron.ballman s AttrDocs description of cpu_dispatch
bodies.
https://reviews.llvm.org/D47474
Files:
include/clang/AST/Decl.h
include/clang/Basic/Attr.td
include/clang/Basic/AttrDocs.td
include/clang/Basic/DiagnosticGroups.td
include/clang/Basic/DiagnosticSemaKinds.td
include/clang/Basic/TargetInfo.h
include/clang/Basic/X86Target.def
lib/AST/Decl.cpp
lib/Basic/Targets/X86.cpp
lib/Basic/Targets/X86.h
lib/CodeGen/CGBuiltin.cpp
lib/CodeGen/CodeGenFunction.cpp
lib/CodeGen/CodeGenFunction.h
lib/CodeGen/CodeGenModule.cpp
lib/CodeGen/CodeGenModule.h
lib/Parse/ParseDecl.cpp
lib/Sema/AnalysisBasedWarnings.cpp
lib/Sema/Sema.cpp
lib/Sema/SemaDecl.cpp
lib/Sema/SemaDeclAttr.cpp
lib/Sema/SemaExpr.cpp
lib/Sema/SemaOverload.cpp
test/CodeGen/attr-cpuspecific.c
test/Misc/pragma-attribute-supported-attributes-list.test
test/Sema/attr-cpuspecific.c
test/SemaCXX/attr-cpuspecific.cpp
utils/TableGen/ClangAttrEmitter.cpp
Index: utils/TableGen/ClangAttrEmitter.cpp
===
--- utils/TableGen/ClangAttrEmitter.cpp
+++ utils/TableGen/ClangAttrEmitter.cpp
@@ -1173,6 +1173,13 @@
}
};
+ class VariadicIdentifierArgument : public VariadicArgument {
+ public:
+VariadicIdentifierArgument(const Record , StringRef Attr)
+ : VariadicArgument(Arg, Attr, "IdentifierInfo *")
+{}
+ };
+
class VariadicStringArgument : public VariadicArgument {
public:
VariadicStringArgument(const Record , StringRef Attr)
@@ -1278,6 +1285,8 @@
Ptr = llvm::make_unique(Arg, Attr);
else if (ArgName == "ParamIdxArgument")
Ptr = llvm::make_unique(Arg, Attr, "ParamIdx");
+ else if (ArgName == "VariadicIdentifierArgument")
+Ptr = llvm::make_unique(Arg, Attr);
else if (ArgName == "VersionArgument")
Ptr = llvm::make_unique(Arg, Attr);
@@ -2106,6 +2115,34 @@
.Default(false);
}
+static bool isVariadicIdentifierArgument(Record *Arg) {
+ return !Arg->getSuperClasses().empty() &&
+ llvm::StringSwitch(
+ Arg->getSuperClasses().back().first->getName())
+ .Case("VariadicIdentifierArgument", true)
+ .Default(false);
+}
+
+static void emitClangAttrVariadicIdentifierArgList(RecordKeeper ,
+ raw_ostream ) {
+ OS << "#if defined(CLANG_ATTR_VARIADIC_IDENTIFIER_ARG_LIST)\n";
+ std::vector Attrs = Records.getAllDerivedDefinitions("Attr");
+ for (const auto *A : Attrs) {
+// Determine whether the first argument is a variadic identifier.
+std::vector Args = A->getValueAsListOfDefs("Args");
+if (Args.empty() || !isVariadicIdentifierArgument(Args[0]))
+ continue;
+
+// All these spellings take an identifier argument.
+forEachUniqueSpelling(*A, [&](const FlattenedSpelling ) {
+ OS << ".Case(\"" << S.name() << "\", "
+ << "true"
+ << ")\n";
+});
+ }
+ OS << "#endif // CLANG_ATTR_VARIADIC_IDENTIFIER_ARG_LIST\n\n";
+}
+
// Emits the first-argument-is-identifier property for attributes.
static void emitClangAttrIdentifierArgList(RecordKeeper , raw_ostream ) {
OS << "#if defined(CLANG_ATTR_IDENTIFIER_ARG_LIST)\n";
@@ -3696,6 +3733,7 @@
emitSourceFileHeader("Parser-related llvm::StringSwitch cases", OS);
emitClangAttrArgContextList(Records, OS);
emitClangAttrIdentifierArgList(Records, OS);
+ emitClangAttrVariadicIdentifierArgList(Records, OS);
emitClangAttrTypeArgList(Records, OS);
emitClangAttrLateParsedList(Records, OS);
}
Index: test/SemaCXX/attr-cpuspecific.cpp
===
--- /dev/null
+++ test/SemaCXX/attr-cpuspecific.cpp
@@ -0,0 +1,111 @@
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -fsyntax-only -verify -fexceptions -fcxx-exceptions %s -std=c++14
+
+// expected-error@+1{{invalid option 'invalid' for cpu_dispatch}}
+void __attribute__((cpu_dispatch(atom, invalid))) invalid_cpu();
+
+void __attribute__((cpu_specific(atom))) no_default(void);
+void __attribute__((cpu_specific(sandybridge))) no_default(void);
+
+struct MVReference {
+ int __attribute__((cpu_specific(sandybridge))) bar(void);
+ int __attribute__((cpu_specific(ivybridge))) bar(void);
+ int __attribute__((cpu_specific(sandybridge))) foo(void);
+};
+
+void use1(void){
+ // OK, will fail in the linker, unless another TU provides the cpu_dispatch.
+ no_default();
+
+ // expected-error@+1 {{call to non-static member function without an object argument}}
+ +MVReference::bar;
+ // expected-error@+1 {{call to non-static member function without an object argument}}
+ +MVReference::foo;
+ // expected-error@+1 {{reference to multiversioned function could not be resolved; did you mean to call it?}}
+ ::bar;
+ // expected-error@+1 {{reference to multiversioned function could not be resolved; did you mean to call it?}}
+
[PATCH] D47474: Implement cpu_dispatch/cpu_specific Multiversioning
erichkeane added inline comments.
Comment at: include/clang/Basic/Attr.td:851
+ let Spellings = [GCC<"cpu_specific">];
+ let Args = [VariadicIdentifierArgument<"Cpus">];
+ let Subjects = SubjectList<[Function]>;
aaron.ballman wrote:
> erichkeane wrote:
> > aaron.ballman wrote:
> > > Be sure to add a test for using this attribute with the C++ spelling, as
> > > I'm not certain how well we would parse something like
> > > `[[gnu::cpu_specific(ivybridge)]]` currently (it may just work, however).
> > >
> > > Also, why an identifier instead of a string literal?
> > I'll add it, presumably as 'clang::cpu_specific'. The decision for
> > string-literal vs identifier was made quite a few years before I was here
> > sadly. I believe the EDG FE doesn't make identifiers any more difficult so
> > the implementer here chose to make it that way.
> >
> > In this case, I'd very much like to keep it with the same implementation as
> > ICC, simply because users of this are already familiar with it in this form.
> > In this case, I'd very much like to keep it with the same implementation as
> > ICC, simply because users of this are already familiar with it in this form.
>
> The compatibility with ICC is important for the GNU-style attribute, but for
> the C++ spelling this is novel territory where there is no compatibility
> story. Another approach to consider is whether to accept identifiers or
> string literals depending on the spelling, but that might not be worth it.
I'd like to think about that... I could forsee accepting BOTH forms, simply
because it would slightly simplify the conversion from an attribute-target-mv
situation, though I'm not sure it is important enough to do.
Comment at: include/clang/Basic/AttrDocs.td:225-226
+as a parameter (like ``cpu_specific``). However, ``cpu_dispatch`` functions
+may not have a body in its definition. An empty definition is permissible for
+ICC compatibility, and all other definitions will have their body ignored.
+
aaron.ballman wrote:
> erichkeane wrote:
> > aaron.ballman wrote:
> > > How empty is empty? e.g., is it lexically empty (no tokens whatsoever) or
> > > notionally empty (empty after preprocessing)?
> > > ```
> > > __attribute__((cpu_dispatch(atom))) void multi_cpu_1(void) {
> > > /* Is this empty enough? */
> > > }
> > >
> > > __attribute__((cpu_dispatch(atom))) void multi_cpu_2(void) {
> > > #if 0
> > > #error How about this?
> > > #endif
> > > }
> > > ```
> > Well, ANY are permissible (even non-empty bodies...) since I issue a
> > warning if I detect that it is not empty. I detect it at the AST level, so
> > anything that doesn't add to the AST isn't counted against 'empty'. In
> > this case, those two are both empty.
> >
> > Do you have a suggestion on how I can word this more clearly?
> Would this be accurate?
> ```
> Functions marked with ``cpu_dispatch`` are not expected to be defined, only
> declared. If such a marked function has a definition, any side effects of the
> function are ignored; trivial function bodies are permissible for ICC
> compatibility.
> ```
> (Question: if this is true, should you also update
> `FunctionDecl::hasTrivialBody()` to return `true` for functions with this
> attribute?)
That is accurate and elegantly describes the behavior. I don't think it makes
sense to mark the body 'trivial' since it actually WILL have a body, its just
emitted based on the attribute rather than the user-defined body. I'd also
fear some of the side-effects of permitting someone to detect it as 'trivial'
for SFINAE purposes.
https://reviews.llvm.org/D47474
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47474: Implement cpu_dispatch/cpu_specific Multiversioning
aaron.ballman added inline comments.
Comment at: include/clang/Basic/Attr.td:851
+ let Spellings = [GCC<"cpu_specific">];
+ let Args = [VariadicIdentifierArgument<"Cpus">];
+ let Subjects = SubjectList<[Function]>;
erichkeane wrote:
> aaron.ballman wrote:
> > Be sure to add a test for using this attribute with the C++ spelling, as
> > I'm not certain how well we would parse something like
> > `[[gnu::cpu_specific(ivybridge)]]` currently (it may just work, however).
> >
> > Also, why an identifier instead of a string literal?
> I'll add it, presumably as 'clang::cpu_specific'. The decision for
> string-literal vs identifier was made quite a few years before I was here
> sadly. I believe the EDG FE doesn't make identifiers any more difficult so
> the implementer here chose to make it that way.
>
> In this case, I'd very much like to keep it with the same implementation as
> ICC, simply because users of this are already familiar with it in this form.
> In this case, I'd very much like to keep it with the same implementation as
> ICC, simply because users of this are already familiar with it in this form.
The compatibility with ICC is important for the GNU-style attribute, but for
the C++ spelling this is novel territory where there is no compatibility story.
Another approach to consider is whether to accept identifiers or string
literals depending on the spelling, but that might not be worth it.
Comment at: include/clang/Basic/AttrDocs.td:225-226
+as a parameter (like ``cpu_specific``). However, ``cpu_dispatch`` functions
+may not have a body in its definition. An empty definition is permissible for
+ICC compatibility, and all other definitions will have their body ignored.
+
erichkeane wrote:
> aaron.ballman wrote:
> > How empty is empty? e.g., is it lexically empty (no tokens whatsoever) or
> > notionally empty (empty after preprocessing)?
> > ```
> > __attribute__((cpu_dispatch(atom))) void multi_cpu_1(void) {
> > /* Is this empty enough? */
> > }
> >
> > __attribute__((cpu_dispatch(atom))) void multi_cpu_2(void) {
> > #if 0
> > #error How about this?
> > #endif
> > }
> > ```
> Well, ANY are permissible (even non-empty bodies...) since I issue a warning
> if I detect that it is not empty. I detect it at the AST level, so anything
> that doesn't add to the AST isn't counted against 'empty'. In this case,
> those two are both empty.
>
> Do you have a suggestion on how I can word this more clearly?
Would this be accurate?
```
Functions marked with ``cpu_dispatch`` are not expected to be defined, only
declared. If such a marked function has a definition, any side effects of the
function are ignored; trivial function bodies are permissible for ICC
compatibility.
```
(Question: if this is true, should you also update
`FunctionDecl::hasTrivialBody()` to return `true` for functions with this
attribute?)
https://reviews.llvm.org/D47474
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47474: Implement cpu_dispatch/cpu_specific Multiversioning
erichkeane added a comment.
Responding to comments :)
Comment at: include/clang/Basic/Attr.td:850
+def CpuSpecific : InheritableAttr {
+ let Spellings = [GCC<"cpu_specific">];
+ let Args = [VariadicIdentifierArgument<"Cpus">];
aaron.ballman wrote:
> Does GCC really support this spelling? I couldn't find documentation to
> suggest it, and a quick test suggests it's not supported there. Perhaps this
> should use the Clang spelling instead?
Yep, you're right. I'd forgotten about that differentiation here. Thanks!
Comment at: include/clang/Basic/Attr.td:851
+ let Spellings = [GCC<"cpu_specific">];
+ let Args = [VariadicIdentifierArgument<"Cpus">];
+ let Subjects = SubjectList<[Function]>;
aaron.ballman wrote:
> Be sure to add a test for using this attribute with the C++ spelling, as I'm
> not certain how well we would parse something like
> `[[gnu::cpu_specific(ivybridge)]]` currently (it may just work, however).
>
> Also, why an identifier instead of a string literal?
I'll add it, presumably as 'clang::cpu_specific'. The decision for
string-literal vs identifier was made quite a few years before I was here
sadly. I believe the EDG FE doesn't make identifiers any more difficult so the
implementer here chose to make it that way.
In this case, I'd very much like to keep it with the same implementation as
ICC, simply because users of this are already familiar with it in this form.
Comment at: include/clang/Basic/AttrDocs.td:225-226
+as a parameter (like ``cpu_specific``). However, ``cpu_dispatch`` functions
+may not have a body in its definition. An empty definition is permissible for
+ICC compatibility, and all other definitions will have their body ignored.
+
aaron.ballman wrote:
> How empty is empty? e.g., is it lexically empty (no tokens whatsoever) or
> notionally empty (empty after preprocessing)?
> ```
> __attribute__((cpu_dispatch(atom))) void multi_cpu_1(void) {
> /* Is this empty enough? */
> }
>
> __attribute__((cpu_dispatch(atom))) void multi_cpu_2(void) {
> #if 0
> #error How about this?
> #endif
> }
> ```
Well, ANY are permissible (even non-empty bodies...) since I issue a warning if
I detect that it is not empty. I detect it at the AST level, so anything that
doesn't add to the AST isn't counted against 'empty'. In this case, those two
are both empty.
Do you have a suggestion on how I can word this more clearly?
https://reviews.llvm.org/D47474
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47474: Implement cpu_dispatch/cpu_specific Multiversioning
erichkeane updated this revision to Diff 149145.
erichkeane marked 11 inline comments as done.
erichkeane added a comment.
Fix based on @craig.topper and @aaron.ballman s comments.
https://reviews.llvm.org/D47474
Files:
include/clang/AST/Decl.h
include/clang/Basic/Attr.td
include/clang/Basic/AttrDocs.td
include/clang/Basic/DiagnosticGroups.td
include/clang/Basic/DiagnosticSemaKinds.td
include/clang/Basic/TargetInfo.h
include/clang/Basic/X86Target.def
lib/AST/Decl.cpp
lib/Basic/Targets/X86.cpp
lib/Basic/Targets/X86.h
lib/CodeGen/CGBuiltin.cpp
lib/CodeGen/CodeGenFunction.cpp
lib/CodeGen/CodeGenFunction.h
lib/CodeGen/CodeGenModule.cpp
lib/CodeGen/CodeGenModule.h
lib/Parse/ParseDecl.cpp
lib/Sema/AnalysisBasedWarnings.cpp
lib/Sema/Sema.cpp
lib/Sema/SemaDecl.cpp
lib/Sema/SemaDeclAttr.cpp
lib/Sema/SemaExpr.cpp
lib/Sema/SemaOverload.cpp
test/CodeGen/attr-cpuspecific.c
test/Misc/pragma-attribute-supported-attributes-list.test
test/Sema/attr-cpuspecific.c
test/SemaCXX/attr-cpuspecific.cpp
utils/TableGen/ClangAttrEmitter.cpp
Index: utils/TableGen/ClangAttrEmitter.cpp
===
--- utils/TableGen/ClangAttrEmitter.cpp
+++ utils/TableGen/ClangAttrEmitter.cpp
@@ -1173,6 +1173,13 @@
}
};
+ class VariadicIdentifierArgument : public VariadicArgument {
+ public:
+VariadicIdentifierArgument(const Record , StringRef Attr)
+ : VariadicArgument(Arg, Attr, "IdentifierInfo *")
+{}
+ };
+
class VariadicStringArgument : public VariadicArgument {
public:
VariadicStringArgument(const Record , StringRef Attr)
@@ -1278,6 +1285,8 @@
Ptr = llvm::make_unique(Arg, Attr);
else if (ArgName == "ParamIdxArgument")
Ptr = llvm::make_unique(Arg, Attr, "ParamIdx");
+ else if (ArgName == "VariadicIdentifierArgument")
+Ptr = llvm::make_unique(Arg, Attr);
else if (ArgName == "VersionArgument")
Ptr = llvm::make_unique(Arg, Attr);
@@ -2106,6 +2115,34 @@
.Default(false);
}
+static bool isVariadicIdentifierArgument(Record *Arg) {
+ return !Arg->getSuperClasses().empty() &&
+ llvm::StringSwitch(
+ Arg->getSuperClasses().back().first->getName())
+ .Case("VariadicIdentifierArgument", true)
+ .Default(false);
+}
+
+static void emitClangAttrVariadicIdentifierArgList(RecordKeeper ,
+ raw_ostream ) {
+ OS << "#if defined(CLANG_ATTR_VARIADIC_IDENTIFIER_ARG_LIST)\n";
+ std::vector Attrs = Records.getAllDerivedDefinitions("Attr");
+ for (const auto *A : Attrs) {
+// Determine whether the first argument is a variadic identifier.
+std::vector Args = A->getValueAsListOfDefs("Args");
+if (Args.empty() || !isVariadicIdentifierArgument(Args[0]))
+ continue;
+
+// All these spellings take an identifier argument.
+forEachUniqueSpelling(*A, [&](const FlattenedSpelling ) {
+ OS << ".Case(\"" << S.name() << "\", "
+ << "true"
+ << ")\n";
+});
+ }
+ OS << "#endif // CLANG_ATTR_VARIADIC_IDENTIFIER_ARG_LIST\n\n";
+}
+
// Emits the first-argument-is-identifier property for attributes.
static void emitClangAttrIdentifierArgList(RecordKeeper , raw_ostream ) {
OS << "#if defined(CLANG_ATTR_IDENTIFIER_ARG_LIST)\n";
@@ -3696,6 +3733,7 @@
emitSourceFileHeader("Parser-related llvm::StringSwitch cases", OS);
emitClangAttrArgContextList(Records, OS);
emitClangAttrIdentifierArgList(Records, OS);
+ emitClangAttrVariadicIdentifierArgList(Records, OS);
emitClangAttrTypeArgList(Records, OS);
emitClangAttrLateParsedList(Records, OS);
}
Index: test/SemaCXX/attr-cpuspecific.cpp
===
--- /dev/null
+++ test/SemaCXX/attr-cpuspecific.cpp
@@ -0,0 +1,111 @@
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -fsyntax-only -verify -fexceptions -fcxx-exceptions %s -std=c++14
+
+// expected-error@+1{{invalid option 'invalid' for cpu_dispatch}}
+void __attribute__((cpu_dispatch(atom, invalid))) invalid_cpu();
+
+void __attribute__((cpu_specific(atom))) no_default(void);
+void __attribute__((cpu_specific(sandybridge))) no_default(void);
+
+struct MVReference {
+ int __attribute__((cpu_specific(sandybridge))) bar(void);
+ int __attribute__((cpu_specific(ivybridge))) bar(void);
+ int __attribute__((cpu_specific(sandybridge))) foo(void);
+};
+
+void use1(void){
+ // OK, will fail in the linker, unless another TU provides the cpu_dispatch.
+ no_default();
+
+ // expected-error@+1 {{call to non-static member function without an object argument}}
+ +MVReference::bar;
+ // expected-error@+1 {{call to non-static member function without an object argument}}
+ +MVReference::foo;
+ // expected-error@+1 {{reference to multiversioned function could not be resolved; did you mean to call it?}}
+ ::bar;
+ // expected-error@+1 {{reference to multiversioned function could not be resolved; did you
[PATCH] D47474: Implement cpu_dispatch/cpu_specific Multiversioning
aaron.ballman added inline comments.
Comment at: include/clang/AST/Decl.h:2212-2213
+ bool isCpuDispatchMultiVersion() const;
+ bool isCpuSpecificMultiVersion() const;
+
Pedantic nit: CPU instead of Cpu?
Comment at: include/clang/Basic/Attr.td:850
+def CpuSpecific : InheritableAttr {
+ let Spellings = [GCC<"cpu_specific">];
+ let Args = [VariadicIdentifierArgument<"Cpus">];
Does GCC really support this spelling? I couldn't find documentation to suggest
it, and a quick test suggests it's not supported there. Perhaps this should use
the Clang spelling instead?
Comment at: include/clang/Basic/Attr.td:851
+ let Spellings = [GCC<"cpu_specific">];
+ let Args = [VariadicIdentifierArgument<"Cpus">];
+ let Subjects = SubjectList<[Function]>;
Be sure to add a test for using this attribute with the C++ spelling, as I'm
not certain how well we would parse something like
`[[gnu::cpu_specific(ivybridge)]]` currently (it may just work, however).
Also, why an identifier instead of a string literal?
Comment at: include/clang/Basic/Attr.td:864
+def CpuDispatch : InheritableAttr {
+ let Spellings = [GCC<"cpu_dispatch">];
+ let Args = [VariadicIdentifierArgument<"Cpus">];
Same here.
Comment at: include/clang/Basic/AttrDocs.td:225-226
+as a parameter (like ``cpu_specific``). However, ``cpu_dispatch`` functions
+may not have a body in its definition. An empty definition is permissible for
+ICC compatibility, and all other definitions will have their body ignored.
+
How empty is empty? e.g., is it lexically empty (no tokens whatsoever) or
notionally empty (empty after preprocessing)?
```
__attribute__((cpu_dispatch(atom))) void multi_cpu_1(void) {
/* Is this empty enough? */
}
__attribute__((cpu_dispatch(atom))) void multi_cpu_2(void) {
#if 0
#error How about this?
#endif
}
```
Comment at: lib/Parse/ParseDecl.cpp:209
-/// Determine whether the given attribute has an identifier argument.
+/// Determine whether the given attribute has a variadic identifier argument.
static bool attributeHasIdentifierArg(const IdentifierInfo ) {
This comment seems to be in the wrong place.
Comment at: lib/Sema/SemaDeclAttr.cpp:1877-1878
+static void handleCpuSpecificAttr(Sema , Decl *D, const AttributeList ) {
+ FunctionDecl *FD = D->getAsFunction();
+ assert(FD && "CPU Specific/Dispatch only valid on a Function Decl");
+
Better to just use `cast(D)` here as it already asserts properly.
Comment at: utils/TableGen/ClangAttrEmitter.cpp:2130
+ std::vector Attrs = Records.getAllDerivedDefinitions("Attr");
+ for (const auto *Attr : Attrs) {
+// Determine whether the first argument is a variadic identifier.
Please don't use `Attr` as a local name; it's a type.
https://reviews.llvm.org/D47474
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47474: Implement cpu_dispatch/cpu_specific Multiversioning
craig.topper added inline comments.
Comment at: include/clang/Basic/X86Target.def:295
+CPU_SPECIFIC("pentium_iii", 'H',
+ (1ULL << FEATURE_CMOV | 1ULL << FEATURE_MMX | 1ULL <<
FEATURE_SSE))
+CPU_SPECIFIC("pentium_iii_no_xmm_regs", 'H',
Could we just make the features a comma separated string? Then we wouldn't need
a third version of EmitX86CpuSupports? Yeah it would incur string processing
costs, but is that a big deal?
Comment at: lib/Sema/SemaDecl.cpp:9214
+return MultiVersioning::Target;
+ else if (FD->hasAttr())
+return MultiVersioning::CpuDispatch;
No need for else after return.
Comment at: lib/Sema/SemaDeclAttr.cpp:1901
+const TargetInfo = S.Context.getTargetInfo();
+if (llvm::find_if(Cpus, [CpuName, ](const IdentifierInfo *Cur) {
+ return Target.cpuSpecificManglingCharacter(CpuName) ==
Maybe use llvm::any_of since you don't actually care about the iterator
returned.
https://reviews.llvm.org/D47474
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47474: Implement cpu_dispatch/cpu_specific Multiversioning
erichkeane updated this revision to Diff 148977.
https://reviews.llvm.org/D47474
Files:
include/clang/AST/Decl.h
include/clang/Basic/Attr.td
include/clang/Basic/AttrDocs.td
include/clang/Basic/DiagnosticGroups.td
include/clang/Basic/DiagnosticSemaKinds.td
include/clang/Basic/TargetInfo.h
include/clang/Basic/X86Target.def
lib/AST/Decl.cpp
lib/Basic/Targets/X86.cpp
lib/Basic/Targets/X86.h
lib/CodeGen/CGBuiltin.cpp
lib/CodeGen/CodeGenFunction.cpp
lib/CodeGen/CodeGenFunction.h
lib/CodeGen/CodeGenModule.cpp
lib/CodeGen/CodeGenModule.h
lib/Parse/ParseDecl.cpp
lib/Sema/AnalysisBasedWarnings.cpp
lib/Sema/Sema.cpp
lib/Sema/SemaDecl.cpp
lib/Sema/SemaDeclAttr.cpp
lib/Sema/SemaExpr.cpp
lib/Sema/SemaOverload.cpp
test/CodeGen/attr-cpuspecific.c
test/Misc/pragma-attribute-supported-attributes-list.test
test/Sema/attr-cpuspecific.c
test/SemaCXX/attr-cpuspecific.cpp
utils/TableGen/ClangAttrEmitter.cpp
Index: utils/TableGen/ClangAttrEmitter.cpp
===
--- utils/TableGen/ClangAttrEmitter.cpp
+++ utils/TableGen/ClangAttrEmitter.cpp
@@ -1173,6 +1173,13 @@
}
};
+ class VariadicIdentifierArgument : public VariadicArgument {
+ public:
+VariadicIdentifierArgument(const Record , StringRef Attr)
+ : VariadicArgument(Arg, Attr, "IdentifierInfo *")
+{}
+ };
+
class VariadicStringArgument : public VariadicArgument {
public:
VariadicStringArgument(const Record , StringRef Attr)
@@ -1278,6 +1285,8 @@
Ptr = llvm::make_unique(Arg, Attr);
else if (ArgName == "ParamIdxArgument")
Ptr = llvm::make_unique(Arg, Attr, "ParamIdx");
+ else if (ArgName == "VariadicIdentifierArgument")
+Ptr = llvm::make_unique(Arg, Attr);
else if (ArgName == "VersionArgument")
Ptr = llvm::make_unique(Arg, Attr);
@@ -2106,6 +2115,34 @@
.Default(false);
}
+static bool isVariadicIdentifierArgument(Record *Arg) {
+ return !Arg->getSuperClasses().empty() &&
+ llvm::StringSwitch(
+ Arg->getSuperClasses().back().first->getName())
+ .Case("VariadicIdentifierArgument", true)
+ .Default(false);
+}
+
+static void emitClangAttrVariadicIdentifierArgList(RecordKeeper ,
+ raw_ostream ) {
+ OS << "#if defined(CLANG_ATTR_VARIADIC_IDENTIFIER_ARG_LIST)\n";
+ std::vector Attrs = Records.getAllDerivedDefinitions("Attr");
+ for (const auto *Attr : Attrs) {
+// Determine whether the first argument is a variadic identifier.
+std::vector Args = Attr->getValueAsListOfDefs("Args");
+if (Args.empty() || !isVariadicIdentifierArgument(Args[0]))
+ continue;
+
+// All these spellings take an identifier argument.
+forEachUniqueSpelling(*Attr, [&](const FlattenedSpelling ) {
+ OS << ".Case(\"" << S.name() << "\", "
+ << "true"
+ << ")\n";
+});
+ }
+ OS << "#endif // CLANG_ATTR_VARIADIC_IDENTIFIER_ARG_LIST\n\n";
+}
+
// Emits the first-argument-is-identifier property for attributes.
static void emitClangAttrIdentifierArgList(RecordKeeper , raw_ostream ) {
OS << "#if defined(CLANG_ATTR_IDENTIFIER_ARG_LIST)\n";
@@ -3696,6 +3733,7 @@
emitSourceFileHeader("Parser-related llvm::StringSwitch cases", OS);
emitClangAttrArgContextList(Records, OS);
emitClangAttrIdentifierArgList(Records, OS);
+ emitClangAttrVariadicIdentifierArgList(Records, OS);
emitClangAttrTypeArgList(Records, OS);
emitClangAttrLateParsedList(Records, OS);
}
Index: test/SemaCXX/attr-cpuspecific.cpp
===
--- /dev/null
+++ test/SemaCXX/attr-cpuspecific.cpp
@@ -0,0 +1,108 @@
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -fsyntax-only -verify -fexceptions -fcxx-exceptions %s -std=c++14
+
+// expected-error@+1{{invalid option 'invalid' for cpu_dispatch}}
+void __attribute__((cpu_dispatch(atom, invalid))) invalid_cpu();
+
+void __attribute__((cpu_specific(atom))) no_default(void);
+void __attribute__((cpu_specific(sandybridge))) no_default(void);
+
+struct MVReference {
+ int __attribute__((cpu_specific(sandybridge))) bar(void);
+ int __attribute__((cpu_specific(ivybridge))) bar(void);
+ int __attribute__((cpu_specific(sandybridge))) foo(void);
+};
+
+void use1(void){
+ // OK, will fail in the linker, unless another TU provides the cpu_dispatch.
+ no_default();
+
+ // expected-error@+1 {{call to non-static member function without an object argument}}
+ +MVReference::bar;
+ // expected-error@+1 {{call to non-static member function without an object argument}}
+ +MVReference::foo;
+ // expected-error@+1 {{reference to multiversioned function could not be resolved; did you mean to call it?}}
+ ::bar;
+ // expected-error@+1 {{reference to multiversioned function could not be resolved; did you mean to call it?}}
+ ::foo;
+}
+
+//expected-error@+1 {{attribute 'cpu_specific' multiversioned functions do not yet support
[PATCH] D47474: Implement cpu_dispatch/cpu_specific Multiversioning
erichkeane marked 2 inline comments as done. erichkeane added a comment. Woops, looks like I lost those in the rebase. Thanks for catching them! Repository: rC Clang https://reviews.llvm.org/D47474 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47474: Implement cpu_dispatch/cpu_specific Multiversioning
mgrang added inline comments. Comment at: lib/Sema/SemaDecl.cpp:9440 + // Sort order doesn't matter, it just needs to be consistent. + std::sort(NewParsed.Features.begin(), NewParsed.Features.end()); Please use llvm::sort instead of std::sort. See https://llvm.org/docs/CodingStandards.html#beware-of-non-deterministic-sorting-order-of-equal-elements. Comment at: lib/Sema/SemaDecl.cpp:9526 +NewParsed = NewTA->parse(); +std::sort(NewParsed.Features.begin(), NewParsed.Features.end()); } Please use llvm::sort instead of std::sort. See https://llvm.org/docs/CodingStandards.html#beware-of-non-deterministic-sorting-order-of-equal-elements. Repository: rC Clang https://reviews.llvm.org/D47474 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47474: Implement cpu_dispatch/cpu_specific Multiversioning
erichkeane created this revision.
erichkeane added reviewers: mikerice, echristo, aaron.ballman, gbiv.
Herald added a reviewer: george.burgess.iv.
Herald added a subscriber: mgrang.
As documented here: https://software.intel.com/en-us/node/682969 and
https://software.intel.com/en-us/node/523346. cpu_dispatch multiversioning
is an ICC feature that provides for function multiversioning.
This feature is implemented with two attributes: First, cpu_specific,
which specifies the individual function versions. Second, cpu_dispatch,
which specifies the location of the resolver function and the list of
resolvable functions.
This is valuable since it provides a mechanism where the resolver's TU
can be specified in one location, and the individual implementions
each in their own translation units.
The goal of this patch is to be source-compatible with ICC, so this
implementation diverges from the ICC implementation in a few ways:
1- Linux x86/64 only: This implementation uses ifuncs in order to
properly dispatch functions. This is is a valuable performance benefit
over the ICC implementation. A future patch will be provided to enable
this feature on Windows, but it will obviously more closely fit ICC's
implementation.
2- CPU Identification functions: ICC uses a set of custom functions to identify
the feature list of the host processor. This patch uses the cpu_supports
functionality in order to better align with 'target' multiversioning.
1- cpu_dispatch function def/decl: ICC's cpu_dispatch requires that the function
marked cpu_dispatch be an empty definition. This patch supports that as well,
however declarations are also permitted, since the linker will solve the
issue of multiple emissions.
Repository:
rC Clang
https://reviews.llvm.org/D47474
Files:
include/clang/AST/Decl.h
include/clang/Basic/Attr.td
include/clang/Basic/AttrDocs.td
include/clang/Basic/DiagnosticGroups.td
include/clang/Basic/DiagnosticSemaKinds.td
include/clang/Basic/TargetInfo.h
include/clang/Basic/X86Target.def
lib/AST/Decl.cpp
lib/Basic/Targets/X86.cpp
lib/Basic/Targets/X86.h
lib/CodeGen/CGBuiltin.cpp
lib/CodeGen/CodeGenFunction.cpp
lib/CodeGen/CodeGenFunction.h
lib/CodeGen/CodeGenModule.cpp
lib/CodeGen/CodeGenModule.h
lib/Parse/ParseDecl.cpp
lib/Sema/AnalysisBasedWarnings.cpp
lib/Sema/Sema.cpp
lib/Sema/SemaDecl.cpp
lib/Sema/SemaDeclAttr.cpp
lib/Sema/SemaExpr.cpp
lib/Sema/SemaOverload.cpp
test/CodeGen/attr-cpuspecific.c
test/Misc/pragma-attribute-supported-attributes-list.test
test/Sema/attr-cpuspecific.c
test/SemaCXX/attr-cpuspecific.cpp
utils/TableGen/ClangAttrEmitter.cpp
Index: utils/TableGen/ClangAttrEmitter.cpp
===
--- utils/TableGen/ClangAttrEmitter.cpp
+++ utils/TableGen/ClangAttrEmitter.cpp
@@ -1173,6 +1173,13 @@
}
};
+ class VariadicIdentifierArgument : public VariadicArgument {
+ public:
+VariadicIdentifierArgument(const Record , StringRef Attr)
+ : VariadicArgument(Arg, Attr, "IdentifierInfo *")
+{}
+ };
+
class VariadicStringArgument : public VariadicArgument {
public:
VariadicStringArgument(const Record , StringRef Attr)
@@ -1278,6 +1285,8 @@
Ptr = llvm::make_unique(Arg, Attr);
else if (ArgName == "ParamIdxArgument")
Ptr = llvm::make_unique(Arg, Attr, "ParamIdx");
+ else if (ArgName == "VariadicIdentifierArgument")
+Ptr = llvm::make_unique(Arg, Attr);
else if (ArgName == "VersionArgument")
Ptr = llvm::make_unique(Arg, Attr);
@@ -2106,6 +2115,34 @@
.Default(false);
}
+static bool isVariadicIdentifierArgument(Record *Arg) {
+ return !Arg->getSuperClasses().empty() &&
+ llvm::StringSwitch(
+ Arg->getSuperClasses().back().first->getName())
+ .Case("VariadicIdentifierArgument", true)
+ .Default(false);
+}
+
+static void emitClangAttrVariadicIdentifierArgList(RecordKeeper ,
+ raw_ostream ) {
+ OS << "#if defined(CLANG_ATTR_VARIADIC_IDENTIFIER_ARG_LIST)\n";
+ std::vector Attrs = Records.getAllDerivedDefinitions("Attr");
+ for (const auto *Attr : Attrs) {
+// Determine whether the first argument is a variadic identifier.
+std::vector Args = Attr->getValueAsListOfDefs("Args");
+if (Args.empty() || !isVariadicIdentifierArgument(Args[0]))
+ continue;
+
+// All these spellings take an identifier argument.
+forEachUniqueSpelling(*Attr, [&](const FlattenedSpelling ) {
+ OS << ".Case(\"" << S.name() << "\", "
+ << "true"
+ << ")\n";
+});
+ }
+ OS << "#endif // CLANG_ATTR_VARIADIC_IDENTIFIER_ARG_LIST\n\n";
+}
+
// Emits the first-argument-is-identifier property for attributes.
static void emitClangAttrIdentifierArgList(RecordKeeper , raw_ostream ) {
OS << "#if defined(CLANG_ATTR_IDENTIFIER_ARG_LIST)\n";
@@ -3696,6 +3733,7 @@
emitSourceFileHeader("Parser-related llvm::StringSwitch
