Fix diag for read-only target features

[yaxunl]

Summary

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.

[arsenm]

clang/test/OpenMP/driver-openmp-amdgpu.c

8 ↗

(On Diff #532094)

Can you use -verify and expected-no-diagnostics? CHECK-NOT is really fragile

[yaxunl]

clang/test/OpenMP/driver-openmp-amdgpu.c

8 ↗

(On Diff #532094)

-verify only works with clang -cc1, whereas we want to make sure there is no warning for the whole compilation invoked by clang driver

[jdoerfert]

clang/test/OpenMP/driver-openmp-amdgpu.c

8 ↗

(On Diff #532094)

let's then check-not for any warning?

[jhuber6]

clang/test/OpenMP/driver-openmp-amdgpu.c

8 ↗

(On Diff #532094)

It's probably reasonable that this shouldn't return any random errors so I'm fine with false positives.

[yaxunl]

use -verify in test

clang/lib/Basic/TargetInfo.cpp

bool TargetInfo::initFeatureMap(
     const std::vector<std::string> &FeatureVec) const {
   for (const auto &F : FeatureVec) {
     StringRef Name = F;
     if (Name.empty())
       continue;
     // 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] == '+');
   }
   return true;
 }
 
 ParsedTargetAttr TargetInfo::parseTargetAttr(StringRef Features) const {
   ParsedTargetAttr Ret;

clang/lib/Basic/Targets.cpp

 #include "Targets/Sparc.h"
 #include "Targets/SystemZ.h"
 #include "Targets/TCE.h"
 #include "Targets/VE.h"
 #include "Targets/WebAssembly.h"
 #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"
 
 using namespace clang;
 
 namespace clang {
 namespace targets {
 //===----------------------------------------------------------------------===//
TargetInfo::CreateTargetInfo(DiagnosticsEngine &Diags,
   // Set the fp math unit.
   if (!Opts->FPMath.empty() && !Target->setFPMath(Opts->FPMath)) {
     Diags.Report(diag::err_target_unknown_fpmath) << Opts->FPMath;
     return nullptr;
   }
 
   // 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;
 
   // Add the features to the compile options.
   Opts->Features.clear();
   for (const auto &F : Opts->FeatureMap)
     Opts->Features.push_back((F.getValue() ? "+" : "-") + F.getKey().str());

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

+// 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