[clang] c8c5830 - Fix diag for read-only target features

[Yaxun Liu via cfe-commits]

Author: Yaxun (Sam) Liu
Date: 2023-06-16T17:48:27-04:00
New Revision: c8c583071591dbc6927955e608dcd44c0bac8b9c

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

LOG: Fix diag for read-only target features

Currently the diag is emitted even when there is no
target feature specified on command line for OpenMP.
This is because the function to initialize feature map
is also used with cached feature string. The fix is to
only diag when the feature map is initialized with
feature strings from command line options.

Reviewed by: Joseph Huber, Matt Arsenault, Johannes Doerfert

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

Added: 
    clang/test/OpenMP/openmp-read-only-feature.c

Modified: 
    clang/lib/Basic/TargetInfo.cpp
    clang/lib/Basic/Targets.cpp

Removed: 
    


################################################################################
diff  --git a/clang/lib/Basic/TargetInfo.cpp b/clang/lib/Basic/TargetInfo.cpp
index 3f0c9d672f718..6cd5d618a4aca 100644
--- a/clang/lib/Basic/TargetInfo.cpp
+++ b/clang/lib/Basic/TargetInfo.cpp
@@ -528,8 +528,6 @@ bool TargetInfo::initFeatureMap(
     // Apply the feature via the target.
     if (Name[0] != '+' && Name[0] != '-')
       Diags.Report(diag::warn_fe_backend_invalid_feature_flag) << Name;
-    else if (isReadOnlyFeature(Name.substr(1)))
-      Diags.Report(diag::warn_fe_backend_readonly_feature_flag) << Name;
     else
       setFeatureEnabled(Features, Name.substr(1), Name[0] == '+');
   }

diff  --git a/clang/lib/Basic/Targets.cpp b/clang/lib/Basic/Targets.cpp
index ab4bdd0930136..636b59fd12725 100644
--- a/clang/lib/Basic/Targets.cpp
+++ b/clang/lib/Basic/Targets.cpp
@@ -42,6 +42,7 @@
 #include "Targets/X86.h"
 #include "Targets/XCore.h"
 #include "clang/Basic/Diagnostic.h"
+#include "clang/Basic/DiagnosticFrontend.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/TargetParser/Triple.h"
 
@@ -816,6 +817,13 @@ TargetInfo::CreateTargetInfo(DiagnosticsEngine &Diags,
 
   // Compute the default target features, we need the target to handle this
   // because features may have dependencies on one another.
+  llvm::erase_if(Opts->FeaturesAsWritten, [&](StringRef Name) {
+    if (Target->isReadOnlyFeature(Name.substr(1))) {
+      Diags.Report(diag::warn_fe_backend_readonly_feature_flag) << Name;
+      return true;
+    }
+    return false;
+  });
   if (!Target->initFeatureMap(Opts->FeatureMap, Diags, Opts->CPU,
                               Opts->FeaturesAsWritten))
     return nullptr;

diff  --git a/clang/test/OpenMP/openmp-read-only-feature.c b/clang/test/OpenMP/openmp-read-only-feature.c
new file mode 100644
index 0000000000000..051ffea05c358
--- /dev/null
+++ b/clang/test/OpenMP/openmp-read-only-feature.c
@@ -0,0 +1,16 @@
+// REQUIRES: x86-registered-target
+// REQUIRES: amdgpu-registered-target
+// REQUIRES: clang-target-64-bits
+
+// RUN: %clang_cc1 -triple amdgcn-amd-amdhsa -target-cpu gfx1030 \
+// RUN:   -fopenmp -nogpulib -fopenmp-is-device -verify %s
+// expected-no-diagnostics
+
+// RUN: %clang_cc1 -triple amdgcn-amd-amdhsa -target-cpu gfx1030 \
+// RUN:   -fopenmp -nogpulib -target-feature -image-insts \
+// RUN:   -fopenmp-is-device -emit-llvm -S -o - %s 2>&1 | FileCheck %s
+// CHECK: warning: feature flag '-image-insts' is ignored since the feature is read only
+
+#pragma omp begin declare variant match(device = {arch(amdgcn)})
+void foo();
+#pragma omp end declare variant