saiislam 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:
> 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?
Isn't giving a remark better than a warning, when we know in many cases this
will be hit during a normal (expected) compilation for target offload?
Remark diagnostic will give ample handle for understanding the flow without the
need to explicitly deal with this warning during compilation of user programs.
I am fine changing it to a warning if you feel strongly about this.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D116549/new/
https://reviews.llvm.org/D116549
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
