erichkeane closed this revision.
erichkeane marked 2 inline comments as done.
erichkeane added a comment.
Closed with revision 322028
https://reviews.llvm.org/D40819
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
rsmith accepted this revision.
rsmith added a comment.
Looks good to me with just a few more tweaks (assuming these comments don't
uncover any new issues). Thank you!
Comment at: lib/AST/ASTContext.cpp:9490
+ assert(FD->isMultiVersion() && "Only valid for multiversioned
erichkeane updated this revision to Diff 128950.
erichkeane marked 2 inline comments as done.
erichkeane added a comment.
All of @rsmith 's comments on the last patch. 1 "open" item on the duplicate
discover in the resolver generation, but hopefully this otherwise acceptable.
erichkeane marked 7 inline comments as done.
erichkeane added a comment.
patch incoming!
Comment at: include/clang/AST/ASTContext.h:2643-2648
+for (auto *CurDecl :
+ FD->getDeclContext()->getRedeclContext()->lookup(FD->getDeclName())) {
+ FunctionDecl *CurFD =
rsmith added a comment.
One other test I'd like to see is what happens if you declare a version before
the first use, and define it afterwards, particularly if the version is an
inline function:
inline __attribute__((target("default"))) void f();
inline __attribute__((target("foo"))) void
erichkeane updated this revision to Diff 128792.
erichkeane added a comment.
This revision is now accepted and ready to land.
Added test for friend functions.
https://reviews.llvm.org/D40819
Files:
include/clang/AST/ASTContext.h
include/clang/AST/Decl.h
include/clang/Basic/Attr.td
erichkeane planned changes to this revision.
erichkeane added a comment.
Forgot friend functions, Working on it now, sorry about that.
https://reviews.llvm.org/D40819
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
erichkeane updated this revision to Diff 128782.
erichkeane marked 7 inline comments as done.
erichkeane added a comment.
Fixes for all @echristo and @rsmith s comments.
https://reviews.llvm.org/D40819
Files:
include/clang/AST/ASTContext.h
include/clang/AST/Decl.h
erichkeane marked 34 inline comments as done.
erichkeane added a comment.
Patch incoming, sorry it took so long!
Comment at: lib/CodeGen/CGBuiltin.cpp:7673
-Value *CodeGenFunction::EmitX86CpuInit() {
+Value *CodeGenFunction::EmitX86CpuInit(CGBuilderTy ) {
rsmith added a comment.
Lots of comments, but no high-level design concerns. I think this is very close
to being ready to go.
Comment at: include/clang/AST/Decl.h:2162-2168
+ /// Sets the multiversion state for this declaration and all of its
+ /// redeclarations.
+ void
echristo accepted this revision.
echristo added a comment.
This revision is now accepted and ready to land.
Couple of inline comments, otherwise I'm pretty happy. I'd wait for an ack by
Richard for this though.
-eric
Comment at: lib/CodeGen/CGBuiltin.cpp:7673
-Value
erichkeane added a comment.
Hi all-- I'm intending to miss the branch for 6.0, but I'd love to get this in
soon after. Can anyone take another look?
Thanks,
Erich
https://reviews.llvm.org/D40819
___
cfe-commits mailing list
erichkeane added a comment.
I'm sure most of you have taken off for the year, but if anyone had time,
*ping* :D
https://reviews.llvm.org/D40819
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
erichkeane updated this revision to Diff 127005.
erichkeane added a comment.
Fix error message per @aaron.ballman s suggestion.
https://reviews.llvm.org/D40819
Files:
include/clang/AST/Decl.h
include/clang/Basic/Attr.td
include/clang/Basic/DiagnosticSemaKinds.td
erichkeane added inline comments.
Comment at: include/clang/Basic/DiagnosticSemaKinds.td:9333
+def err_multiversion_duplicate : Error<
+ "multiversion function duplicate declarations require identical target "
+ "attributes">;
aaron.ballman wrote:
> instead of
aaron.ballman added inline comments.
Comment at: include/clang/Basic/DiagnosticSemaKinds.td:9333-9335
+ "multiversion function would have identical mangling to a previous "
+ "definition. Duplicate declarations must have identical target attribute "
+ "values">;
erichkeane updated this revision to Diff 126829.
erichkeane added a comment.
@aaron.ballman reminded me that this should only work on ELF targets, so this
patch enforces that constraint.
https://reviews.llvm.org/D40819
Files:
include/clang/AST/Decl.h
include/clang/Basic/Attr.td
erichkeane updated this revision to Diff 126781.
erichkeane marked 2 inline comments as done.
erichkeane added a comment.
Fixed all of @aaron.ballman comments.
https://reviews.llvm.org/D40819
Files:
include/clang/AST/Decl.h
include/clang/Basic/Attr.td
erichkeane marked 17 inline comments as done.
erichkeane added a comment.
Patch incoming, Thank you very much for the review @aaron.ballman
Comment at: include/clang/Basic/DiagnosticSemaKinds.td:9333-9335
+ "multiversion function would have identical mangling to a previous "
aaron.ballman added inline comments.
Comment at: include/clang/Basic/Attr.td:1809
bool DuplicateArchitecture = false;
+ bool operator==(const ParsedTargetAttr ) {
+return DuplicateArchitecture == Other.DuplicateArchitecture &&
This function
erichkeane added a comment.
Herald added a subscriber: mgrang.
Ping! Anyone have time to take a look for me?
https://reviews.llvm.org/D40819
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
erichkeane updated this revision to Diff 125779.
erichkeane marked 3 inline comments as done.
erichkeane added a comment.
Fix all rsmith's and craig's fixes, AFAIK.
https://reviews.llvm.org/D40819
Files:
include/clang/AST/Decl.h
include/clang/Basic/Attr.td
erichkeane marked 14 inline comments as done.
erichkeane added a comment.
Incoming patch!
Comment at: include/clang/Basic/Attr.td:1809
bool DuplicateArchitecture = false;
+ bool operator ==(const ParsedTargetAttr ) {
+return DuplicateArchitecture ==
rsmith added a comment.
I expect there are more places that need to be changed, but this is looking
surprisingly clean.
You will need to teach the modules merging code that it needs to check this
attribute in addition to checking that types match when considering merging
declaration chains
craig.topper added inline comments.
Comment at: include/clang/Basic/X86Target.def:148
/// Cannonlake client microarchitecture based processors.
PROC(Cannonlake, "cannonlake", PROC_64_BIT)
Cannonlake can prioritize off of VBMI or IFMA.
erichkeane created this revision.
GCC's attribute 'target', in addition to being an optimization hint,
also allows function multiversioning. We currently have the former
implemented, this is the latter's implementation.
This works by enabling functions with the same name/signature to coexist,
so
26 matches
Mail list logo
