[PATCH] D83281: [OpenMP] Allow traits for the OpenMP context selector `isa`

2020-11-19 Thread Johannes Doerfert via Phabricator via cfe-commits 
jdoerfert added inline comments.



Comment at: clang/include/clang/AST/OpenMPClause.h:7641
+  /// (which accepts anything) and (later) extensions.
+  StringRef RawString;
 };

mikerice wrote:
> This field doesn't seem to have serialization code.  Is that expected or an 
> oversight?
Oversight. I assume we need it to preserve the behavior when we go the pch 
route. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83281

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D83281: [OpenMP] Allow traits for the OpenMP context selector `isa`

2020-11-19 Thread Mike Rice via Phabricator via cfe-commits 
mikerice added inline comments.



Comment at: clang/include/clang/AST/OpenMPClause.h:7641
+  /// (which accepts anything) and (later) extensions.
+  StringRef RawString;
 };

This field doesn't seem to have serialization code.  Is that expected or an 
oversight?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83281

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D83281: [OpenMP] Allow traits for the OpenMP context selector `isa`

2020-07-29 Thread Nico Weber via Phabricator via cfe-commits 
thakis added a comment.

That did the trick, thanks!

The bot was broken for > 5h and 43 builds. We got lucky that nothing else broke 
in the meantime, but red bots make it more difficult to analyze new regressions 
that appear when the bots are already red. Next time, please revert while you 
investigate / until you have time to investigate.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83281

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D83281: [OpenMP] Allow traits for the OpenMP context selector `isa`

2020-07-29 Thread Johannes Doerfert via Phabricator via cfe-commits 
jdoerfert added a comment.

In D83281#2183100 , @thakis wrote:

> Looks like it's still failing with that change: 
> http://45.33.8.238/win/20885/step_7.txt

dso_local is *before* the void,.. 2 min


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83281

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D83281: [OpenMP] Allow traits for the OpenMP context selector `isa`

2020-07-29 Thread Nico Weber via Phabricator via cfe-commits 
thakis added a comment.

Looks like it's still failing with that change: 
http://45.33.8.238/win/20885/step_7.txt


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83281

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D83281: [OpenMP] Allow traits for the OpenMP context selector `isa`

2020-07-29 Thread Johannes Doerfert via Phabricator via cfe-commits 
jdoerfert added a comment.

In D83281#2182507 , @thakis wrote:

> This breaks check-clang on Windows: http://45.33.8.238/win/20850/step_7.txt
>
> Please take a look, and revert while you investigate if it takes a while to 
> fix.

Should be fixed by 8723280b68b1e5ed97a699466720b36a32a9e406 
, 
apologies for the delay.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83281

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D83281: [OpenMP] Allow traits for the OpenMP context selector `isa`

2020-07-29 Thread Nico Weber via Phabricator via cfe-commits 
thakis added a comment.

This breaks check-clang on Windows: http://45.33.8.238/win/20850/step_7.txt

Please take a look, and revert while you investigate if it takes a while to fix.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83281

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D83281: [OpenMP] Allow traits for the OpenMP context selector `isa`

2020-07-29 Thread Johannes Doerfert via Phabricator via cfe-commits 
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGee05167cc42b: [OpenMP] Allow traits for the OpenMP context 
selector `isa` (authored by jdoerfert).

Changed prior to commit:
  https://reviews.llvm.org/D83281?vs=280327=281606#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83281

Files:
  clang/include/clang/AST/OpenMPClause.h
  clang/include/clang/Basic/DiagnosticParseKinds.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/AST/OpenMPClause.cpp
  clang/lib/Parse/ParseOpenMP.cpp
  clang/lib/Sema/SemaOpenMP.cpp
  clang/test/OpenMP/declare_variant_device_isa_codegen_1.c
  clang/test/OpenMP/declare_variant_messages.c
  llvm/include/llvm/Frontend/OpenMP/OMPContext.h
  llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
  llvm/lib/Frontend/OpenMP/OMPContext.cpp
  llvm/unittests/Frontend/OpenMPContextTest.cpp

Index: llvm/unittests/Frontend/OpenMPContextTest.cpp
===
--- llvm/unittests/Frontend/OpenMPContextTest.cpp
+++ llvm/unittests/Frontend/OpenMPContextTest.cpp
@@ -38,11 +38,13 @@
 #define OMP_TRAIT_PROPERTY(Enum, TraitSetEnum, TraitSelectorEnum, Str) \
   EXPECT_EQ(TraitProperty::Enum,   \
 getOpenMPContextTraitPropertyKind( \
-TraitSet::TraitSetEnum,\
-getOpenMPContextTraitPropertyName(TraitProperty::Enum)));  \
+TraitSet::TraitSetEnum, TraitSelector::TraitSelectorEnum,  \
+getOpenMPContextTraitPropertyName(TraitProperty::Enum, Str))); \
   EXPECT_EQ(Str, getOpenMPContextTraitPropertyName(\
- getOpenMPContextTraitPropertyKind(TraitSet::TraitSetEnum, \
-   Str))); \
+ getOpenMPContextTraitPropertyKind(\
+ TraitSet::TraitSetEnum,   \
+ TraitSelector::TraitSelectorEnum, Str),   \
+ Str));\
   EXPECT_EQ(TraitSet::TraitSetEnum,\
 getOpenMPContextTraitSetForProperty(TraitProperty::Enum)); \
   EXPECT_EQ(TraitSelector::TraitSelectorEnum,  \
@@ -77,31 +79,31 @@
   EXPECT_TRUE(isVariantApplicableInContext(Empty, DeviceNVPTX));
 
   VariantMatchInfo UserCondFalse;
-  UserCondFalse.addTrait(TraitProperty::user_condition_false);
+  UserCondFalse.addTrait(TraitProperty::user_condition_false, "");
   EXPECT_FALSE(isVariantApplicableInContext(UserCondFalse, HostLinux));
   EXPECT_FALSE(isVariantApplicableInContext(UserCondFalse, DeviceLinux));
   EXPECT_FALSE(isVariantApplicableInContext(UserCondFalse, HostNVPTX));
   EXPECT_FALSE(isVariantApplicableInContext(UserCondFalse, DeviceNVPTX));
 
   VariantMatchInfo DeviceArchArm;
-  DeviceArchArm.addTrait(TraitProperty::device_arch_arm);
+  DeviceArchArm.addTrait(TraitProperty::device_arch_arm, "");
   EXPECT_FALSE(isVariantApplicableInContext(DeviceArchArm, HostLinux));
   EXPECT_FALSE(isVariantApplicableInContext(DeviceArchArm, DeviceLinux));
   EXPECT_FALSE(isVariantApplicableInContext(DeviceArchArm, HostNVPTX));
   EXPECT_FALSE(isVariantApplicableInContext(DeviceArchArm, DeviceNVPTX));
 
   VariantMatchInfo LLVMHostUserCondTrue;
-  LLVMHostUserCondTrue.addTrait(TraitProperty::implementation_vendor_llvm);
-  LLVMHostUserCondTrue.addTrait(TraitProperty::device_kind_host);
-  LLVMHostUserCondTrue.addTrait(TraitProperty::device_kind_any);
-  LLVMHostUserCondTrue.addTrait(TraitProperty::user_condition_true);
+  LLVMHostUserCondTrue.addTrait(TraitProperty::implementation_vendor_llvm, "");
+  LLVMHostUserCondTrue.addTrait(TraitProperty::device_kind_host, "");
+  LLVMHostUserCondTrue.addTrait(TraitProperty::device_kind_any, "");
+  LLVMHostUserCondTrue.addTrait(TraitProperty::user_condition_true, "");
   EXPECT_TRUE(isVariantApplicableInContext(LLVMHostUserCondTrue, HostLinux));
   EXPECT_FALSE(isVariantApplicableInContext(LLVMHostUserCondTrue, DeviceLinux));
   EXPECT_TRUE(isVariantApplicableInContext(LLVMHostUserCondTrue, HostNVPTX));
   EXPECT_FALSE(isVariantApplicableInContext(LLVMHostUserCondTrue, DeviceNVPTX));
 
   VariantMatchInfo LLVMHostUserCondTrueCPU = LLVMHostUserCondTrue;
-  LLVMHostUserCondTrueCPU.addTrait(TraitProperty::device_kind_cpu);
+  LLVMHostUserCondTrueCPU.addTrait(TraitProperty::device_kind_cpu, "");
   EXPECT_TRUE(isVariantApplicableInContext(LLVMHostUserCondTrueCPU, HostLinux));
   EXPECT_FALSE(
   isVariantApplicableInContext(LLVMHostUserCondTrueCPU, DeviceLinux));
@@ -111,14

[PATCH] D83281: [OpenMP] Allow traits for the OpenMP context selector `isa`

2020-07-29 Thread Alexey Bataev via Phabricator via cfe-commits 
ABataev accepted this revision.
ABataev added a comment.
This revision is now accepted and ready to land.

LG with a nit.




Comment at: clang/lib/AST/OpenMPClause.cpp:2304-2306
+  auto it = FeatureMap.find(RawString);
+  if (it != FeatureMap.end())
+return it->second;

`It`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83281

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D83281: [OpenMP] Allow traits for the OpenMP context selector `isa`

2020-07-28 Thread Johannes Doerfert via Phabricator via cfe-commits 
jdoerfert added a comment.

ping :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83281

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D83281: [OpenMP] Allow traits for the OpenMP context selector `isa`

2020-07-23 Thread Johannes Doerfert via Phabricator via cfe-commits 
jdoerfert updated this revision to Diff 280327.
jdoerfert added a comment.

minor fixes


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83281

Files:
  clang/include/clang/AST/OpenMPClause.h
  clang/include/clang/Basic/DiagnosticParseKinds.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/AST/OpenMPClause.cpp
  clang/lib/Parse/ParseOpenMP.cpp
  clang/lib/Sema/SemaOpenMP.cpp
  clang/test/OpenMP/declare_variant_device_isa_codegen_1.c
  clang/test/OpenMP/declare_variant_messages.c
  llvm/include/llvm/Frontend/OpenMP/OMPContext.h
  llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
  llvm/lib/Frontend/OpenMP/OMPContext.cpp
  llvm/unittests/Frontend/OpenMPContextTest.cpp

Index: llvm/unittests/Frontend/OpenMPContextTest.cpp
===
--- llvm/unittests/Frontend/OpenMPContextTest.cpp
+++ llvm/unittests/Frontend/OpenMPContextTest.cpp
@@ -38,11 +38,13 @@
 #define OMP_TRAIT_PROPERTY(Enum, TraitSetEnum, TraitSelectorEnum, Str) \
   EXPECT_EQ(TraitProperty::Enum,   \
 getOpenMPContextTraitPropertyKind( \
-TraitSet::TraitSetEnum,\
-getOpenMPContextTraitPropertyName(TraitProperty::Enum)));  \
+TraitSet::TraitSetEnum, TraitSelector::TraitSelectorEnum,  \
+getOpenMPContextTraitPropertyName(TraitProperty::Enum, Str))); \
   EXPECT_EQ(Str, getOpenMPContextTraitPropertyName(\
- getOpenMPContextTraitPropertyKind(TraitSet::TraitSetEnum, \
-   Str))); \
+ getOpenMPContextTraitPropertyKind(\
+ TraitSet::TraitSetEnum,   \
+ TraitSelector::TraitSelectorEnum, Str),   \
+ Str));\
   EXPECT_EQ(TraitSet::TraitSetEnum,\
 getOpenMPContextTraitSetForProperty(TraitProperty::Enum)); \
   EXPECT_EQ(TraitSelector::TraitSelectorEnum,  \
@@ -77,31 +79,31 @@
   EXPECT_TRUE(isVariantApplicableInContext(Empty, DeviceNVPTX));
 
   VariantMatchInfo UserCondFalse;
-  UserCondFalse.addTrait(TraitProperty::user_condition_false);
+  UserCondFalse.addTrait(TraitProperty::user_condition_false, "");
   EXPECT_FALSE(isVariantApplicableInContext(UserCondFalse, HostLinux));
   EXPECT_FALSE(isVariantApplicableInContext(UserCondFalse, DeviceLinux));
   EXPECT_FALSE(isVariantApplicableInContext(UserCondFalse, HostNVPTX));
   EXPECT_FALSE(isVariantApplicableInContext(UserCondFalse, DeviceNVPTX));
 
   VariantMatchInfo DeviceArchArm;
-  DeviceArchArm.addTrait(TraitProperty::device_arch_arm);
+  DeviceArchArm.addTrait(TraitProperty::device_arch_arm, "");
   EXPECT_FALSE(isVariantApplicableInContext(DeviceArchArm, HostLinux));
   EXPECT_FALSE(isVariantApplicableInContext(DeviceArchArm, DeviceLinux));
   EXPECT_FALSE(isVariantApplicableInContext(DeviceArchArm, HostNVPTX));
   EXPECT_FALSE(isVariantApplicableInContext(DeviceArchArm, DeviceNVPTX));
 
   VariantMatchInfo LLVMHostUserCondTrue;
-  LLVMHostUserCondTrue.addTrait(TraitProperty::implementation_vendor_llvm);
-  LLVMHostUserCondTrue.addTrait(TraitProperty::device_kind_host);
-  LLVMHostUserCondTrue.addTrait(TraitProperty::device_kind_any);
-  LLVMHostUserCondTrue.addTrait(TraitProperty::user_condition_true);
+  LLVMHostUserCondTrue.addTrait(TraitProperty::implementation_vendor_llvm, "");
+  LLVMHostUserCondTrue.addTrait(TraitProperty::device_kind_host, "");
+  LLVMHostUserCondTrue.addTrait(TraitProperty::device_kind_any, "");
+  LLVMHostUserCondTrue.addTrait(TraitProperty::user_condition_true, "");
   EXPECT_TRUE(isVariantApplicableInContext(LLVMHostUserCondTrue, HostLinux));
   EXPECT_FALSE(isVariantApplicableInContext(LLVMHostUserCondTrue, DeviceLinux));
   EXPECT_TRUE(isVariantApplicableInContext(LLVMHostUserCondTrue, HostNVPTX));
   EXPECT_FALSE(isVariantApplicableInContext(LLVMHostUserCondTrue, DeviceNVPTX));
 
   VariantMatchInfo LLVMHostUserCondTrueCPU = LLVMHostUserCondTrue;
-  LLVMHostUserCondTrueCPU.addTrait(TraitProperty::device_kind_cpu);
+  LLVMHostUserCondTrueCPU.addTrait(TraitProperty::device_kind_cpu, "");
   EXPECT_TRUE(isVariantApplicableInContext(LLVMHostUserCondTrueCPU, HostLinux));
   EXPECT_FALSE(
   isVariantApplicableInContext(LLVMHostUserCondTrueCPU, DeviceLinux));
@@ -111,14 +113,14 @@
   isVariantApplicableInContext(LLVMHostUserCondTrueCPU, DeviceNVPTX));
 
   VariantMatchInfo GPU;
-  GPU.addTrait(TraitProperty::device_kind_gpu);
+  GPU.addTrait(TraitProperty::device_kind_gpu, "");

[PATCH] D83281: [OpenMP] Allow traits for the OpenMP context selector `isa`

2020-07-23 Thread Johannes Doerfert via Phabricator via cfe-commits 
jdoerfert updated this revision to Diff 280151.
jdoerfert marked an inline comment as done.
jdoerfert added a comment.

Add virtual


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83281

Files:
  clang/include/clang/AST/OpenMPClause.h
  clang/include/clang/Basic/DiagnosticParseKinds.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/AST/OpenMPClause.cpp
  clang/lib/Parse/ParseOpenMP.cpp
  clang/lib/Sema/SemaOpenMP.cpp
  clang/test/OpenMP/declare_variant_device_isa_codegen_1.c
  clang/test/OpenMP/declare_variant_messages.c
  llvm/include/llvm/Frontend/OpenMP/OMPContext.h
  llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
  llvm/lib/Frontend/OpenMP/OMPContext.cpp
  llvm/unittests/Frontend/OpenMPContextTest.cpp

Index: llvm/unittests/Frontend/OpenMPContextTest.cpp
===
--- llvm/unittests/Frontend/OpenMPContextTest.cpp
+++ llvm/unittests/Frontend/OpenMPContextTest.cpp
@@ -38,11 +38,13 @@
 #define OMP_TRAIT_PROPERTY(Enum, TraitSetEnum, TraitSelectorEnum, Str) \
   EXPECT_EQ(TraitProperty::Enum,   \
 getOpenMPContextTraitPropertyKind( \
-TraitSet::TraitSetEnum,\
-getOpenMPContextTraitPropertyName(TraitProperty::Enum)));  \
+TraitSet::TraitSetEnum, TraitSelector::TraitSelectorEnum,  \
+getOpenMPContextTraitPropertyName(TraitProperty::Enum, Str))); \
   EXPECT_EQ(Str, getOpenMPContextTraitPropertyName(\
- getOpenMPContextTraitPropertyKind(TraitSet::TraitSetEnum, \
-   Str))); \
+ getOpenMPContextTraitPropertyKind(\
+ TraitSet::TraitSetEnum,   \
+ TraitSelector::TraitSelectorEnum, Str),   \
+ Str));\
   EXPECT_EQ(TraitSet::TraitSetEnum,\
 getOpenMPContextTraitSetForProperty(TraitProperty::Enum)); \
   EXPECT_EQ(TraitSelector::TraitSelectorEnum,  \
Index: llvm/lib/Frontend/OpenMP/OMPContext.cpp
===
--- llvm/lib/Frontend/OpenMP/OMPContext.cpp
+++ llvm/lib/Frontend/OpenMP/OMPContext.cpp
@@ -175,11 +175,11 @@
 LLVM_DEBUG({
   if (MK == MK_ALL)
 dbgs() << "[" << DEBUG_TYPE << "] Property "
-   << getOpenMPContextTraitPropertyName(Property)
+   << getOpenMPContextTraitPropertyName(Property, "")
<< " was not in the OpenMP context but match kind is all.\n";
   if (MK == MK_NONE)
 dbgs() << "[" << DEBUG_TYPE << "] Property "
-   << getOpenMPContextTraitPropertyName(Property)
+   << getOpenMPContextTraitPropertyName(Property, "")
<< " was in the OpenMP context but match kind is none.\n";
 });
 return false;
@@ -198,6 +198,14 @@
   continue;
 
 bool IsActiveTrait = Ctx.ActiveTraits.test(unsigned(Property));
+
+// We overwrite the isa trait as it is actually up to the OMPContext hook to
+// check the raw string(s).
+if (Property == TraitProperty::device_isa___ANY)
+  IsActiveTrait = llvm::all_of(VMI.ISATraits, [&](StringRef RawString) {
+return Ctx.matchesISATrait(RawString);
+  });
+
 Optional Result = HandleTrait(Property, IsActiveTrait);
 if (Result.hasValue())
   return Result.getValue();
@@ -225,7 +233,7 @@
 
   if (!FoundInOrder) {
 LLVM_DEBUG(dbgs() << "[" << DEBUG_TYPE << "] Construct property "
-  << getOpenMPContextTraitPropertyName(Property)
+  << getOpenMPContextTraitPropertyName(Property, "")
   << " was not nested properly.\n");
 return false;
   }
@@ -425,8 +433,12 @@
   llvm_unreachable("Unknown trait selector!");
 }
 
-TraitProperty llvm::omp::getOpenMPContextTraitPropertyKind(TraitSet Set,
-   StringRef S) {
+TraitProperty llvm::omp::getOpenMPContextTraitPropertyKind(
+TraitSet Set, TraitSelector Selector, StringRef S) {
+  // Special handling for `device={isa(...)}` as we accept anything here. It is
+  // up to the target to decide if the feature is available.
+  if (Set == TraitSet::device && Selector == TraitSelector::device_isa)
+return TraitProperty::device_isa___ANY;
 #define OMP_TRAIT_PROPERTY(Enum, TraitSetEnum, TraitSelectorEnum, Str) \
   if (Set == TraitSet::TraitSetEnum && Str == S)   \
 return

[PATCH] D83281: [OpenMP] Allow traits for the OpenMP context selector `isa`

2020-07-23 Thread Alexey Bataev via Phabricator via cfe-commits 
ABataev added inline comments.



Comment at: clang/include/clang/AST/OpenMPClause.h:7663
+   const FunctionDecl *CurrentFunctionDecl);
+  ~TargetOMPContext() = default;
+

`virtual`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83281



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D83281: [OpenMP] Allow traits for the OpenMP context selector `isa`

2020-07-23 Thread Johannes Doerfert via Phabricator via cfe-commits 
jdoerfert updated this revision to Diff 280148.
jdoerfert marked 6 inline comments as done.
jdoerfert added a comment.

Addressed @ABataev comments, thx!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83281

Files:
  clang/include/clang/AST/OpenMPClause.h
  clang/include/clang/Basic/DiagnosticParseKinds.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/AST/OpenMPClause.cpp
  clang/lib/Parse/ParseOpenMP.cpp
  clang/lib/Sema/SemaOpenMP.cpp
  clang/test/OpenMP/declare_variant_device_isa_codegen_1.c
  clang/test/OpenMP/declare_variant_messages.c
  llvm/include/llvm/Frontend/OpenMP/OMPContext.h
  llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
  llvm/lib/Frontend/OpenMP/OMPContext.cpp
  llvm/unittests/Frontend/OpenMPContextTest.cpp

Index: llvm/unittests/Frontend/OpenMPContextTest.cpp
===
--- llvm/unittests/Frontend/OpenMPContextTest.cpp
+++ llvm/unittests/Frontend/OpenMPContextTest.cpp
@@ -38,11 +38,13 @@
 #define OMP_TRAIT_PROPERTY(Enum, TraitSetEnum, TraitSelectorEnum, Str) \
   EXPECT_EQ(TraitProperty::Enum,   \
 getOpenMPContextTraitPropertyKind( \
-TraitSet::TraitSetEnum,\
-getOpenMPContextTraitPropertyName(TraitProperty::Enum)));  \
+TraitSet::TraitSetEnum, TraitSelector::TraitSelectorEnum,  \
+getOpenMPContextTraitPropertyName(TraitProperty::Enum, Str))); \
   EXPECT_EQ(Str, getOpenMPContextTraitPropertyName(\
- getOpenMPContextTraitPropertyKind(TraitSet::TraitSetEnum, \
-   Str))); \
+ getOpenMPContextTraitPropertyKind(\
+ TraitSet::TraitSetEnum,   \
+ TraitSelector::TraitSelectorEnum, Str),   \
+ Str));\
   EXPECT_EQ(TraitSet::TraitSetEnum,\
 getOpenMPContextTraitSetForProperty(TraitProperty::Enum)); \
   EXPECT_EQ(TraitSelector::TraitSelectorEnum,  \
Index: llvm/lib/Frontend/OpenMP/OMPContext.cpp
===
--- llvm/lib/Frontend/OpenMP/OMPContext.cpp
+++ llvm/lib/Frontend/OpenMP/OMPContext.cpp
@@ -175,11 +175,11 @@
 LLVM_DEBUG({
   if (MK == MK_ALL)
 dbgs() << "[" << DEBUG_TYPE << "] Property "
-   << getOpenMPContextTraitPropertyName(Property)
+   << getOpenMPContextTraitPropertyName(Property, "")
<< " was not in the OpenMP context but match kind is all.\n";
   if (MK == MK_NONE)
 dbgs() << "[" << DEBUG_TYPE << "] Property "
-   << getOpenMPContextTraitPropertyName(Property)
+   << getOpenMPContextTraitPropertyName(Property, "")
<< " was in the OpenMP context but match kind is none.\n";
 });
 return false;
@@ -198,6 +198,14 @@
   continue;
 
 bool IsActiveTrait = Ctx.ActiveTraits.test(unsigned(Property));
+
+// We overwrite the isa trait as it is actually up to the OMPContext hook to
+// check the raw string(s).
+if (Property == TraitProperty::device_isa___ANY)
+  IsActiveTrait = llvm::all_of(VMI.ISATraits, [&](StringRef RawString) {
+return Ctx.matchesISATrait(RawString);
+  });
+
 Optional Result = HandleTrait(Property, IsActiveTrait);
 if (Result.hasValue())
   return Result.getValue();
@@ -225,7 +233,7 @@
 
   if (!FoundInOrder) {
 LLVM_DEBUG(dbgs() << "[" << DEBUG_TYPE << "] Construct property "
-  << getOpenMPContextTraitPropertyName(Property)
+  << getOpenMPContextTraitPropertyName(Property, "")
   << " was not nested properly.\n");
 return false;
   }
@@ -425,8 +433,12 @@
   llvm_unreachable("Unknown trait selector!");
 }
 
-TraitProperty llvm::omp::getOpenMPContextTraitPropertyKind(TraitSet Set,
-   StringRef S) {
+TraitProperty llvm::omp::getOpenMPContextTraitPropertyKind(
+TraitSet Set, TraitSelector Selector, StringRef S) {
+  // Special handling for `device={isa(...)}` as we accept anything here. It is
+  // up to the target to decide if the feature is available.
+  if (Set == TraitSet::device && Selector == TraitSelector::device_isa)
+return TraitProperty::device_isa___ANY;
 #define OMP_TRAIT_PROPERTY(Enum, TraitSetEnum, TraitSelectorEnum, Str) \
   if (Set == TraitSet::TraitSetEnum && Str == S)   \

[PATCH] D83281: [OpenMP] Allow traits for the OpenMP context selector `isa`

2020-07-23 Thread Alexey Bataev via Phabricator via cfe-commits 
ABataev added inline comments.



Comment at: clang/include/clang/AST/OpenMPClause.h:7599
+  /// (which accept anything) and (later) extensions.
+  StringRef RawString{};
 };

No need for the default initializer here



Comment at: clang/include/clang/AST/OpenMPClause.h:7658
+/// Clang specific specialization of the OMPContext to lookup target features.
+struct TargetOMPContext : public llvm::omp::OMPContext {
+

`final` and virtual default destructor?



Comment at: clang/include/clang/AST/OpenMPClause.h:7665
+  /// See llvm::omp::OMPContext::matchesISATrait
+  bool matchesISATrait(StringRef RawString) const;
+

`override`?



Comment at: clang/lib/Parse/ParseOpenMP.cpp:1826
+
+std::function DiagUnknownTrait = [&](StringRef ISATrait) {
+  // TODO Track the selector locations in a way that is accessible here to

Seems to me, `SourceLocation` is usually captured by value.



Comment at: clang/lib/Sema/SemaOpenMP.cpp:5894
   ASTContext  = getASTContext();
-  OMPContext OMPCtx(getLangOpts().OpenMPIsDevice,
-Context.getTargetInfo().getTriple());
+  std::function DiagUnknownTrait = [&](StringRef ISATrait) {
+// TODO Track the selector locations in a way that is accessible here to

`CE` can be captured by value



Comment at: llvm/lib/Frontend/OpenMP/OMPContext.cpp:205
+if (Property == TraitProperty::device_isa___ANY)
+  IsActiveTrait = std::all_of(
+  VMI.ISATraits.begin(), VMI.ISATraits.end(),

`llvm::all_of`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83281



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D83281: [OpenMP] Allow traits for the OpenMP context selector `isa`

2020-07-23 Thread Johannes Doerfert via Phabricator via cfe-commits 
jdoerfert added a comment.

ping :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83281



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D83281: [OpenMP] Allow traits for the OpenMP context selector `isa`

2020-07-15 Thread Johannes Doerfert via Phabricator via cfe-commits 
jdoerfert updated this revision to Diff 278175.
jdoerfert added a comment.

Add warning and negative test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83281

Files:
  clang/include/clang/AST/OpenMPClause.h
  clang/include/clang/Basic/DiagnosticParseKinds.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/AST/OpenMPClause.cpp
  clang/lib/Parse/ParseOpenMP.cpp
  clang/lib/Sema/SemaOpenMP.cpp
  clang/test/OpenMP/declare_variant_device_isa_codegen_1.c
  clang/test/OpenMP/declare_variant_messages.c
  llvm/include/llvm/Frontend/OpenMP/OMPContext.h
  llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
  llvm/lib/Frontend/OpenMP/OMPContext.cpp
  llvm/unittests/Frontend/OpenMPContextTest.cpp

Index: llvm/unittests/Frontend/OpenMPContextTest.cpp
===
--- llvm/unittests/Frontend/OpenMPContextTest.cpp
+++ llvm/unittests/Frontend/OpenMPContextTest.cpp
@@ -38,11 +38,13 @@
 #define OMP_TRAIT_PROPERTY(Enum, TraitSetEnum, TraitSelectorEnum, Str) \
   EXPECT_EQ(TraitProperty::Enum,   \
 getOpenMPContextTraitPropertyKind( \
-TraitSet::TraitSetEnum,\
-getOpenMPContextTraitPropertyName(TraitProperty::Enum)));  \
+TraitSet::TraitSetEnum, TraitSelector::TraitSelectorEnum,  \
+getOpenMPContextTraitPropertyName(TraitProperty::Enum, Str))); \
   EXPECT_EQ(Str, getOpenMPContextTraitPropertyName(\
- getOpenMPContextTraitPropertyKind(TraitSet::TraitSetEnum, \
-   Str))); \
+ getOpenMPContextTraitPropertyKind(\
+ TraitSet::TraitSetEnum,   \
+ TraitSelector::TraitSelectorEnum, Str),   \
+ Str));\
   EXPECT_EQ(TraitSet::TraitSetEnum,\
 getOpenMPContextTraitSetForProperty(TraitProperty::Enum)); \
   EXPECT_EQ(TraitSelector::TraitSelectorEnum,  \
Index: llvm/lib/Frontend/OpenMP/OMPContext.cpp
===
--- llvm/lib/Frontend/OpenMP/OMPContext.cpp
+++ llvm/lib/Frontend/OpenMP/OMPContext.cpp
@@ -175,11 +175,11 @@
 LLVM_DEBUG({
   if (MK == MK_ALL)
 dbgs() << "[" << DEBUG_TYPE << "] Property "
-   << getOpenMPContextTraitPropertyName(Property)
+   << getOpenMPContextTraitPropertyName(Property, "")
<< " was not in the OpenMP context but match kind is all.\n";
   if (MK == MK_NONE)
 dbgs() << "[" << DEBUG_TYPE << "] Property "
-   << getOpenMPContextTraitPropertyName(Property)
+   << getOpenMPContextTraitPropertyName(Property, "")
<< " was in the OpenMP context but match kind is none.\n";
 });
 return false;
@@ -198,6 +198,14 @@
   continue;
 
 bool IsActiveTrait = Ctx.ActiveTraits.test(unsigned(Property));
+
+// We overwrite the isa trait as it is actually up to the OMPContext hook to
+// check the raw string(s).
+if (Property == TraitProperty::device_isa___ANY)
+  IsActiveTrait = std::all_of(
+  VMI.ISATraits.begin(), VMI.ISATraits.end(),
+  [&](StringRef RawString) { return Ctx.matchesISATrait(RawString); });
+
 Optional Result = HandleTrait(Property, IsActiveTrait);
 if (Result.hasValue())
   return Result.getValue();
@@ -225,7 +233,7 @@
 
   if (!FoundInOrder) {
 LLVM_DEBUG(dbgs() << "[" << DEBUG_TYPE << "] Construct property "
-  << getOpenMPContextTraitPropertyName(Property)
+  << getOpenMPContextTraitPropertyName(Property, "")
   << " was not nested properly.\n");
 return false;
   }
@@ -425,8 +433,12 @@
   llvm_unreachable("Unknown trait selector!");
 }
 
-TraitProperty llvm::omp::getOpenMPContextTraitPropertyKind(TraitSet Set,
-   StringRef S) {
+TraitProperty llvm::omp::getOpenMPContextTraitPropertyKind(
+TraitSet Set, TraitSelector Selector, StringRef S) {
+  // Special handling for `device={isa(...)}` as we accept anything here. It is
+  // up to the target to decide if the feature is available.
+  if (Set == TraitSet::device && Selector == TraitSelector::device_isa)
+return TraitProperty::device_isa___ANY;
 #define OMP_TRAIT_PROPERTY(Enum, TraitSetEnum, TraitSelectorEnum, Str) \
   if (Set == TraitSet::TraitSetEnum && Str == S)   \
 return

[PATCH] D83281: [OpenMP] Allow traits for the OpenMP context selector `isa`

2020-07-14 Thread Jon Chesterfield via Phabricator via cfe-commits 
JonChesterfield added a comment.

Agreed on tests. I like the mechanism - passing a string through to the backend 
as a way to dispatch between isa properties looks cleanly extensible. We 
probably do want to emit a warning when the backend claims it doesn't know 
anything about said string as it'll be prone to typos.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83281



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D83281: [OpenMP] Allow traits for the OpenMP context selector `isa`

2020-07-07 Thread Johannes Doerfert via Phabricator via cfe-commits 
jdoerfert created this revision.
jdoerfert added reviewers: jhuber6, fghanim, JonChesterfield, grokos, 
ggeorgakoudis, ABataev.
Herald added subscribers: llvm-commits, sstefan1, guansong, bollu, hiraditya, 
yaxunl.
Herald added projects: clang, LLVM.

NOTE: The changes are fairly mechanical overall but this patch currently lacks
proper testing. An updated version with more test coverage will be
provided.

It was unclear what `isa` was supposed to mean so we did not provide any
traits for this context selector. With this patch we will allow *any*
string or identifier. We use the target attribute and target info to
determine if the trait matches. In other words, we will check if the
provided value is a target feature that is available (at the call site).

Fixes PR46338


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D83281

Files:
  clang/include/clang/AST/OpenMPClause.h
  clang/lib/AST/OpenMPClause.cpp
  clang/lib/Parse/ParseOpenMP.cpp
  clang/lib/Sema/SemaOpenMP.cpp
  clang/test/OpenMP/declare_variant_device_isa_codegen_1.c
  llvm/include/llvm/Frontend/OpenMP/OMPContext.h
  llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
  llvm/lib/Frontend/OpenMP/OMPContext.cpp
  llvm/unittests/Frontend/OpenMPContextTest.cpp

Index: llvm/unittests/Frontend/OpenMPContextTest.cpp
===
--- llvm/unittests/Frontend/OpenMPContextTest.cpp
+++ llvm/unittests/Frontend/OpenMPContextTest.cpp
@@ -38,11 +38,13 @@
 #define OMP_TRAIT_PROPERTY(Enum, TraitSetEnum, TraitSelectorEnum, Str) \
   EXPECT_EQ(TraitProperty::Enum,   \
 getOpenMPContextTraitPropertyKind( \
-TraitSet::TraitSetEnum,\
-getOpenMPContextTraitPropertyName(TraitProperty::Enum)));  \
+TraitSet::TraitSetEnum, TraitSelector::TraitSelectorEnum,  \
+getOpenMPContextTraitPropertyName(TraitProperty::Enum, Str))); \
   EXPECT_EQ(Str, getOpenMPContextTraitPropertyName(\
- getOpenMPContextTraitPropertyKind(TraitSet::TraitSetEnum, \
-   Str))); \
+ getOpenMPContextTraitPropertyKind(\
+ TraitSet::TraitSetEnum,   \
+ TraitSelector::TraitSelectorEnum, Str),   \
+ Str));\
   EXPECT_EQ(TraitSet::TraitSetEnum,\
 getOpenMPContextTraitSetForProperty(TraitProperty::Enum)); \
   EXPECT_EQ(TraitSelector::TraitSelectorEnum,  \
Index: llvm/lib/Frontend/OpenMP/OMPContext.cpp
===
--- llvm/lib/Frontend/OpenMP/OMPContext.cpp
+++ llvm/lib/Frontend/OpenMP/OMPContext.cpp
@@ -175,11 +175,11 @@
 LLVM_DEBUG({
   if (MK == MK_ALL)
 dbgs() << "[" << DEBUG_TYPE << "] Property "
-   << getOpenMPContextTraitPropertyName(Property)
+   << getOpenMPContextTraitPropertyName(Property, "")
<< " was not in the OpenMP context but match kind is all.\n";
   if (MK == MK_NONE)
 dbgs() << "[" << DEBUG_TYPE << "] Property "
-   << getOpenMPContextTraitPropertyName(Property)
+   << getOpenMPContextTraitPropertyName(Property, "")
<< " was in the OpenMP context but match kind is none.\n";
 });
 return false;
@@ -198,6 +198,14 @@
   continue;
 
 bool IsActiveTrait = Ctx.ActiveTraits.test(unsigned(Property));
+
+// We overwrite the isa trait as it is actually up to the OMPContext hook to
+// check the raw string(s).
+if (Property == TraitProperty::device_isa___ANY)
+  IsActiveTrait = std::all_of(
+  VMI.ISATraits.begin(), VMI.ISATraits.end(),
+  [&](StringRef RawString) { return Ctx.matchesISATrait(RawString); });
+
 Optional Result = HandleTrait(Property, IsActiveTrait);
 if (Result.hasValue())
   return Result.getValue();
@@ -225,7 +233,7 @@
 
   if (!FoundInOrder) {
 LLVM_DEBUG(dbgs() << "[" << DEBUG_TYPE << "] Construct property "
-  << getOpenMPContextTraitPropertyName(Property)
+  << getOpenMPContextTraitPropertyName(Property, "")
   << " was not nested properly.\n");
 return false;
   }
@@ -425,8 +433,12 @@
   llvm_unreachable("Unknown trait selector!");
 }
 
-TraitProperty llvm::omp::getOpenMPContextTraitPropertyKind(TraitSet Set,
-   StringRef S) {
+TraitProperty llvm::omp::getOpenMPContextTraitPropertyKind(
+TraitSet Set, TraitSelector Selector,