[PATCH] D77414: [OpenMP] Add match_{all,any,none} declare variant selector extensions.

Mike Rice via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Sun Apr 5 17:06:20 PDT 2020


mikerice added inline comments.


================
Comment at: clang/lib/Parse/ParseOpenMP.cpp:1823
     ASTContext &ASTCtx = Actions.getASTContext();
-    TI.getAsVariantMatchInfo(ASTCtx, VMI, /* DeviceSetOnly */ true);
+    TI.getAsVariantMatchInfo(ASTCtx, VMI);
     OMPContext OMPCtx(ASTCtx.getLangOpts().OpenMPIsDevice,
----------------
One of the lit tests fails because this is called before semantic checks on the score expression.  This function tries to evaluate the score as a constant and if it isn't will crash.  Probably not specific to this change but how can we deal with that?

```
int foo(void);
#pragma omp begin declare variant match(implementation={vendor(score(foo()) ibm)})
#pragma omp end declare variant
```


================
Comment at: llvm/lib/Frontend/OpenMP/OMPContext.cpp:161
+                          bool WasFound) -> Optional<bool> /* Result */ {
+    // For kind "any" a single match is enough but we ignore non-matched properties.
+    if (MK == MK_ANY) {
----------------
Line too long?


================
Comment at: llvm/lib/Frontend/OpenMP/OMPContext.cpp:252
+                                             bool DeviceSetOnly) {
+  return isVariantApplicableInContextHelper(VMI, Ctx, nullptr, DeviceSetOnly);
 }
----------------
Comment on nullptr would be nice.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D77414/new/

https://reviews.llvm.org/D77414





More information about the cfe-commits mailing list