r239583 - Add a warning for unsupported elements of the target attribute.

[Eric Christopher]

Hi Aaron,

I've committed the patch with the quoting in r239635.

As we spoke on irc we're not doing individual string checking on these two
substrings of possible program strings so I'm going to leave them as it is,
if we do end up checking or matching then yes, I agree a StringSwitch would
make sense.

Thanks a ton for the review :)

-eric

On Fri, Jun 12, 2015 at 6:03 AM Aaron Ballman <aaron at aaronballman.com>
wrote:

> On Thu, Jun 11, 2015 at 9:36 PM, Eric Christopher <echristo at gmail.com>
> wrote:
> > Author: echristo
> > Date: Thu Jun 11 20:36:05 2015
> > New Revision: 239583
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=239583&view=rev
> > Log:
> > Add a warning for unsupported elements of the target attribute.
> >
> > Since we're ignoring the tune= and fpmath= attributes go ahead
> > and add a warning alerting people to the fact that we're going
> > to ignore that part of it during code generation and tie it to
> > the attribute warning set.
> >
> > Modified:
> >     cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
> >     cfe/trunk/include/clang/Sema/Sema.h
> >     cfe/trunk/lib/Sema/SemaDeclAttr.cpp
> >     cfe/trunk/test/Sema/attr-target.c
> >
> > Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
> > URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=239583&r1=239582&r2=239583&view=diff
> >
> ==============================================================================
> > --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
> > +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Thu Jun 11
> 20:36:05 2015
> > @@ -1974,8 +1974,11 @@ def err_attribute_too_few_arguments : Er
> >  def err_attribute_invalid_vector_type : Error<"invalid vector element
> type %0">;
> >  def err_attribute_bad_neon_vector_size : Error<
> >    "Neon vector size must be 64 or 128 bits">;
> > -def err_attribute_unsupported : Error<
> > -  "%0 attribute is not supported for this target">;
> > +def warn_unsupported_target_attribute
> > +    : Warning<"Ignoring unsupported %0 in the target attribute string">,
> > +    InGroup<IgnoredAttributes>;
>
> Please ensure that %0 is quoted properly.
>
> > +def err_attribute_unsupported
> > +    : Error<"%0 attribute is not supported for this target">;
> >  // The err_*_attribute_argument_not_int are seperate because they're
> used by
> >  // VerifyIntegerConstantExpression.
> >  def err_aligned_attribute_argument_not_int : Error<
> >
> > Modified: cfe/trunk/include/clang/Sema/Sema.h
> > URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=239583&r1=239582&r2=239583&view=diff
> >
> ==============================================================================
> > --- cfe/trunk/include/clang/Sema/Sema.h (original)
> > +++ cfe/trunk/include/clang/Sema/Sema.h Thu Jun 11 20:36:05 2015
> > @@ -2820,6 +2820,7 @@ public:
> >                                        unsigned ArgNum, StringRef &Str,
> >                                        SourceLocation *ArgLocation =
> nullptr);
> >    bool checkSectionName(SourceLocation LiteralLoc, StringRef Str);
> > +  void checkTargetAttr(SourceLocation LiteralLoc, StringRef Str);
> >    bool checkMSInheritanceAttrOnDefinition(
> >        CXXRecordDecl *RD, SourceRange Range, bool BestCase,
> >        MSInheritanceAttr::Spelling SemanticSpelling);
> >
> > Modified: cfe/trunk/lib/Sema/SemaDeclAttr.cpp
> > URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclAttr.cpp?rev=239583&r1=239582&r2=239583&view=diff
> >
> ==============================================================================
> > --- cfe/trunk/lib/Sema/SemaDeclAttr.cpp (original)
> > +++ cfe/trunk/lib/Sema/SemaDeclAttr.cpp Thu Jun 11 20:36:05 2015
> > @@ -2397,14 +2397,22 @@ static void handleSectionAttr(Sema &S, D
> >      D->addAttr(NewAttr);
> >  }
> >
> > +// Check for things we'd like to warn about, no errors or validation
> for now.
> > +// TODO: Validation should use a backend target library that specifies
> > +// the allowable subtarget features and cpus. We could use something
> like a
> > +// TargetCodeGenInfo hook here to do validation.
> > +void Sema::checkTargetAttr(SourceLocation LiteralLoc, StringRef
> AttrStr) {
> > +  for (auto Str : {"tune=", "fpmath="})
> > +    if (AttrStr.find(Str) != StringRef::npos)
> > +      Diag(LiteralLoc, diag::warn_unsupported_target_attribute) << Str;
>
> I know this only has two entries right now and is going to change
> someday, but a StringSwitch would make me happier.
>
> > +}
> > +
> >  static void handleTargetAttr(Sema &S, Decl *D, const AttributeList
> &Attr) {
> > -  // TODO: Validation should use a backend target library that specifies
> > -  // the allowable subtarget features and cpus. We could use something
> like a
> > -  // TargetCodeGenInfo hook here to do validation.
> >    StringRef Str;
> >    SourceLocation LiteralLoc;
> >    if (!S.checkStringLiteralArgumentAttr(Attr, 0, Str, &LiteralLoc))
> >      return;
> > +  S.checkTargetAttr(LiteralLoc, Str);
> >    unsigned Index = Attr.getAttributeSpellingListIndex();
> >    TargetAttr *NewAttr =
> >        ::new (S.Context) TargetAttr(Attr.getRange(), S.Context, Str,
> Index);
> >
> > Modified: cfe/trunk/test/Sema/attr-target.c
> > URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/attr-target.c?rev=239583&r1=239582&r2=239583&view=diff
> >
> ==============================================================================
> > --- cfe/trunk/test/Sema/attr-target.c (original)
> > +++ cfe/trunk/test/Sema/attr-target.c Thu Jun 11 20:36:05 2015
> > @@ -2,5 +2,7 @@
> >
> >  int __attribute__((target("avx,sse4.2,arch=ivybridge"))) foo() { return
> 4; }
> >  int __attribute__((target())) bar() { return 4; } //expected-error
> {{'target' attribute takes one argument}}
> > +int __attribute__((target("tune=sandybridge"))) baz() { return 4; }
> //expected-warning {{Ignoring unsupported tune= in the target attribute
> string}}
> > +int __attribute__((target("fpmath=387"))) walrus() { return 4; }
> //expected-warning {{Ignoring unsupported fpmath= in the target attribute
> string}}
>
> Thanks!
>
> ~Aaron
>
> >
> >
> >
> >
> > _______________________________________________
> > cfe-commits mailing list
> > cfe-commits at cs.uiuc.edu
> > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20150612/9d68000e/attachment.html>