jdoerfert added inline comments.
================
Comment at: clang/include/clang/Serialization/ASTBitCodes.h:1952
- STMT_MS_DEPENDENT_EXISTS, // MSDependentExistsStmt
- EXPR_LAMBDA, // LambdaExpr
STMT_COROUTINE_BODY,
----------------
alokmishra.besu wrote:
> jdoerfert wrote:
> > Unrelated.
> Only STMT_OMP_META_DIRECTIVE was added. Rest was formatted by git clang-format
> Rest was formatted by git clang-format
Please don't format unrelated code.
================
Comment at: clang/lib/Parse/ParseOpenMP.cpp:2264
+ Actions.StartOpenMPDSABlock(DirKind, DirName, Actions.getCurScope(),
+ Loc);
+ int paren = 0;
----------------
alokmishra.besu wrote:
> jdoerfert wrote:
> > Should we not go back to the original code handling "directives" instead?
> > This looks like it is copied here.
> Unfortunately we cannot go to the original code handling since the original
> code handling assumes that the directive always ends with
> annot_pragma_openmp_end, while here it will always end with ')'.
> In specification 5.0, since we are choosing only 1 directive, the body of the
> while block remains the same as the original code. Only the condition of the
> while block changes. In specification 5.1, we will need to generate code for
> dynamic handling and even the body will differ as we might need to generate
> AST node for multiple directives. It is best if we handle this code here for
> easier handling of 5.1 code, than in the original code space.
> I will add a TODO comment here.
> Unfortunately we cannot go to the original code handling since the original
> code handling assumes that the directive always ends with
> annot_pragma_openmp_end, while here it will always end with ')'.
Let's add a flag to the original handling to make this possible then. Copying
it is going to create more long term problems.
================
Comment at: llvm/lib/Frontend/OpenMP/OMPContext.cpp:337
+ const SmallVectorImpl<VariantMatchInfo> &VMIs, const OMPContext &Ctx,
+ SmallVectorImpl<unsigned> *OrderedMatch) {
+
----------------
alokmishra.besu wrote:
> jdoerfert wrote:
> > This looks like a clone of `getBestVariantMatchForContext` with an extra
> > unused argument.
> I intended to keep it similar for 5.0 to be updated in 5.1 code. But anyways
> it seems to be giving wrong result. Will go through this again.
Use the `getBestVariantMatchForContext` interface now and for the 5.1 semantics
we introduce the `OrderedMatch`. That can also be used in the call site declare
variant handling which currently calls `getBestVariantMatchForContext` multiple
times.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D91944/new/
https://reviews.llvm.org/D91944
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
