[clang] ad9e98b - [OpenMP] Do not propagate match extensions to nested contexts

[Johannes Doerfert via cfe-commits]

Author: Johannes Doerfert
Date: 2021-03-11T23:31:21-06:00
New Revision: ad9e98b8efa0138559eb640023695dab54967a8d

URL: https://github.com/llvm/llvm-project/commit/ad9e98b8efa0138559eb640023695dab54967a8d
DIFF: https://github.com/llvm/llvm-project/commit/ad9e98b8efa0138559eb640023695dab54967a8d.diff

LOG: [OpenMP] Do not propagate match extensions to nested contexts

If we have nested declare variant context, it doesn't make sense to
inherit the match extension from the parent. Instead, just skip it.

Reviewed By: JonChesterfield

Differential Revision: https://reviews.llvm.org/D95764

Added: 
    clang/test/OpenMP/begin_declare_variant_nested_propagation.c

Modified: 
    clang/include/clang/Basic/AttrDocs.td
    clang/lib/Parse/ParseOpenMP.cpp
    llvm/lib/Frontend/OpenMP/OMPContext.cpp

Removed: 
    


################################################################################
diff  --git a/clang/include/clang/Basic/AttrDocs.td b/clang/include/clang/Basic/AttrDocs.td
index deda68b64f90..b1a80752b650 100644
--- a/clang/include/clang/Basic/AttrDocs.td
+++ b/clang/include/clang/Basic/AttrDocs.td
@@ -4014,7 +4014,8 @@ The match extensions change when the *entire* context selector is considered a
 match for an OpenMP context. The default is ``all``, with ``none`` no trait in the
 selector is allowed to be in the OpenMP context, with ``any`` a single trait in
 both the selector and OpenMP context is sufficient. Only a single match
-extension trait is allowed per context selector.
+extension trait is allowed per context selector. Note that match extensions are
+not propagated to nested selectors like the standard defines it for other traits.
 The disable extensions remove default effects of the ``begin declare variant``
 applied to a definition. If ``disable_implicit_base`` is given, we will not
 introduce an implicit base function for a variant if no base function was

diff  --git a/clang/lib/Parse/ParseOpenMP.cpp b/clang/lib/Parse/ParseOpenMP.cpp
index 3de01be0db87..dd98e89313df 100644
--- a/clang/lib/Parse/ParseOpenMP.cpp
+++ b/clang/lib/Parse/ParseOpenMP.cpp
@@ -21,6 +21,7 @@
 #include "clang/Parse/RAIIObjectsForParser.h"
 #include "clang/Sema/Scope.h"
 #include "llvm/ADT/PointerIntPair.h"
+#include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/StringSwitch.h"
 #include "llvm/ADT/UniqueVector.h"
 #include "llvm/Frontend/OpenMP/OMPContext.h"
@@ -1463,7 +1464,29 @@ bool Parser::parseOMPDeclareVariantMatchClause(SourceLocation Loc,
   // TODO: Keep some source location in the TI to provide better diagnostics.
   // TODO: Perform some kind of equivalence check on the condition and score
   //       expressions.
-  for (const OMPTraitSet &ParentSet : ParentTI->Sets) {
+  auto StripImplementation = [](const OMPTraitSet &TSet) -> OMPTraitSet {
+    if (TSet.Kind != llvm::omp::TraitSet::implementation)
+      return TSet;
+    OMPTraitSet Set = TSet;
+    for (OMPTraitSelector &Selector : Set.Selectors) {
+      if (Selector.Kind != llvm::omp::TraitSelector::implementation_extension)
+        continue;
+      // Do not propagate match extensions to nested contexts.
+      llvm::erase_if(Selector.Properties, [](const OMPTraitProperty &Property) {
+        return (
+            Property.Kind ==
+                llvm::omp::TraitProperty::implementation_extension_match_any ||
+            Property.Kind ==
+                llvm::omp::TraitProperty::implementation_extension_match_all ||
+            Property.Kind ==
+                llvm::omp::TraitProperty::implementation_extension_match_none);
+      });
+      return Set;
+    }
+    return Set;
+  };
+  for (const OMPTraitSet &PSet : ParentTI->Sets) {
+    const OMPTraitSet ParentSet = StripImplementation(PSet);
     bool MergedSet = false;
     for (OMPTraitSet &Set : TI.Sets) {
       if (Set.Kind != ParentSet.Kind)

diff  --git a/clang/test/OpenMP/begin_declare_variant_nested_propagation.c b/clang/test/OpenMP/begin_declare_variant_nested_propagation.c
new file mode 100644
index 000000000000..e0dee1110922
--- /dev/null
+++ b/clang/test/OpenMP/begin_declare_variant_nested_propagation.c
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 -triple=x86_64-pc-win32 -verify -fopenmp -x c -std=c99 -fms-extensions -Wno-pragma-pack %s
+// expected-no-diagnostics
+
+#pragma omp begin declare variant match(user={condition(1)}, device={kind(cpu)}, implementation={extension(match_any)})
+#pragma omp begin declare variant match(device = {kind(cpu, fpga)})
+ this is never reached
+#pragma omp end declare variant
+#pragma omp end declare variant
+
+#pragma omp begin declare variant match(user={condition(1)}, device={kind(cpu)}, implementation={extension(match_any)})
+#pragma omp begin declare variant match(device = {kind(cpu, fpga)}, implementation={vendor(llvm)})
+ this is never reached
+#pragma omp end declare variant
+#pragma omp end declare variant

diff  --git a/llvm/lib/Frontend/OpenMP/OMPContext.cpp b/llvm/lib/Frontend/OpenMP/OMPContext.cpp
index 11d8da097c6c..3d053c29cbdd 100644
--- a/llvm/lib/Frontend/OpenMP/OMPContext.cpp
+++ b/llvm/lib/Frontend/OpenMP/OMPContext.cpp
@@ -168,8 +168,13 @@ static int isVariantApplicableInContextHelper(
     // For kind "any" a single match is enough but we ignore non-matched
     // properties.
     if (MK == MK_ANY) {
-      if (WasFound)
+      if (WasFound) {
+        LLVM_DEBUG(
+            dbgs() << "[" << DEBUG_TYPE << "] Property "
+                   << getOpenMPContextTraitPropertyName(Property, "")
+                   << " was in the OpenMP context and match kind is any.\n";);
         return true;
+      }
       return None;
     }