[PATCH] D70739: [OPENMP50]Add device/isa context selector support.

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert added a comment.

Thanks for working on this! One nit and one concern below.



================
Comment at: clang/include/clang/Sema/Sema.h:9320
   using OMPCtxSelectorData =
-      OpenMPCtxSelectorData<SmallVector<OMPCtxStringType, 4>, ExprResult>;
+      OpenMPCtxSelectorData<SmallVector<llvm::Any, 4>, ExprResult>;
 
----------------
I would like to avoid the Any type here and I hope we can if we don't allow "strings". See also my last comment.


================
Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:11147
+    llvm::Triple CustomTriple(Name);
+    if (CGM.getTarget().getTriple().getArch() != CustomTriple.getArch())
+      return false;
----------------
should we normalize the names here? at least `.lower` might be useful.


================
Comment at: clang/test/OpenMP/nvptx_declare_variant_device_isa_codegen.cpp:48
+
+#pragma omp declare variant(foo) match(device = {isa("nvptx64")})
+int bar() { return 1; }
----------------
I'm not sure we want these to be strings. I would have assumed `isa(nvptx64)`, similar to `vendor(arm)`, or `kind(host)` and `kind(gpu)`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70739