[PATCH] D116549: [OpenMP][Clang] Allow passing target features in ISA trait for metadirective clause
Johannes Doerfert via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Mon Jan 10 06:45:54 PST 2022
jdoerfert added inline comments.
================
Comment at: clang/lib/Parse/ParseOpenMP.cpp:2533
+ std::function<void(StringRef)> DiagUnknownTrait = [this, Loc](
+ StringRef ISATrait) {};
+ TargetOMPContext OMPCtx(ASTContext, std::move(DiagUnknownTrait),
----------------
jdoerfert wrote:
> saiislam wrote:
> > jdoerfert wrote:
> > > Why doesn't this diagnose nothing?
> > Because an isa-feature will fail at least once, for either host compilation or device compilation. So, no point in always giving a warning.
> That is debatable.
>
> First, if I compile for a single architecture there is no device compilation and it should warn.
> Second, if I place the metadirective into a declare variant function or add a `kind(...)` selector to it it will also not warn even if you have multiple architectures.
>
>
```
ASTContext &Context = getASTContext();
std::function<void(StringRef)> DiagUnknownTrait = [this,
┊ ┊ ┊ ┊ ┊ ┊ ┊ ┊ ┊ ┊ CE](StringRef ISATrait) {
┊ // TODO Track the selector locations in a way that is accessible here to
┊ // improve the diagnostic location.
┊ Diag(CE->getBeginLoc(), diag::warn_unknown_declare_variant_isa_trait)
┊ ┊ ┊ << ISATrait;
};
TargetOMPContext OMPCtx(Context, std::move(DiagUnknownTrait),
┊ ┊ ┊ ┊ ┊ ┊ ┊ ┊ ┊ ┊ getCurFunctionDecl(), DSAStack->getConstructTraits());
```
Already exists (SemaOpenMP). Why do we need a second, different diagnostic?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D116549/new/
https://reviews.llvm.org/D116549
More information about the cfe-commits
mailing list