[PATCH] D35519: Add SEMA checking to attribute 'target'

2017-07-18 Thread Erich Keane via Phabricator via cfe-commits
erichkeane abandoned this revision. erichkeane added a comment. Going to split this into a few patches as Eric requested. https://reviews.llvm.org/D35519 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D35519: Add SEMA checking to attribute 'target'

2017-07-18 Thread Erich Keane via Phabricator via cfe-commits
erichkeane added a comment. In https://reviews.llvm.org/D35519#813082, @echristo wrote: > If you wouldn't mind I'd like to see this separated out a bit - > a) I think we can make the attribute info a struct rather than a pair as a > simple change first > b) setCPU split > c) let's see where

[PATCH] D35519: Add SEMA checking to attribute 'target'

2017-07-18 Thread Eric Christopher via Phabricator via cfe-commits
echristo added a comment. If you wouldn't mind I'd like to see this separated out a bit - a) I think we can make the attribute info a struct rather than a pair as a simple change first b) setCPU split c) let's see where we are after that? Also the description makes it sound like you're adding

[PATCH] D35519: Add SEMA checking to attribute 'target'

2017-07-18 Thread Erich Keane via Phabricator via cfe-commits
erichkeane marked 2 inline comments as done. erichkeane added a comment. Thanks Aaron! Fixed those things and will be submitting soon. Also ran format on the function in SemaDeclAttr. For some reason it didn't right the first time. https://reviews.llvm.org/D35519

[PATCH] D35519: Add SEMA checking to attribute 'target'

2017-07-18 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. A few small nits, but otherwise LGTM Comment at: lib/Sema/SemaDeclAttr.cpp:2999-3000 +bool Sema::checkTargetAttr(SourceLocation LiteralLoc, StringRef AttrStr)

[PATCH] D35519: Add SEMA checking to attribute 'target'

2017-07-18 Thread Erich Keane via Phabricator via cfe-commits
erichkeane updated this revision to Diff 107101. erichkeane marked 9 inline comments as done. erichkeane added a comment. Thanks for the feedback Aaron. This should fix everything you had an issue with. -Erich https://reviews.llvm.org/D35519 Files: include/clang/Basic/Attr.td

[PATCH] D35519: Add SEMA checking to attribute 'target'

2017-07-18 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments. Comment at: include/clang/Basic/DiagnosticSemaKinds.td:2389-2397 def warn_unsupported_target_attribute : Warning<"Ignoring unsupported '%0' in the target attribute string">, InGroup; +def warn_duplicate_target_attribute +:

[PATCH] D35519: Add SEMA checking to attribute 'target'

2017-07-17 Thread Erich Keane via Phabricator via cfe-commits
erichkeane created this revision. Attribute 'target' can take either features or architectures. This patch accomplishes this in a couple of ways: 1- It adds support to Targets.cpp for "isValidCPUName" and "isValidFeatureName". HOWEVER, the latter is only implemented for X86. I'm hoping to