[PATCH] D153123: Fix diag for read-only target features

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl created this revision.
yaxunl added reviewers: tra, jhuber6, arsenm.
Herald added subscribers: kerbowa, jvesely.
Herald added a project: All.
yaxunl requested review of this revision.
Herald added subscribers: jplehr, sstefan1, wdng.
Herald added a reviewer: jdoerfert.

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.


https://reviews.llvm.org/D153123

Files:
  clang/lib/Basic/TargetInfo.cpp
  clang/lib/Basic/Targets.cpp
  clang/test/OpenMP/driver-openmp-amdgpu.c


Index: clang/test/OpenMP/driver-openmp-amdgpu.c
===================================================================
--- /dev/null
+++ clang/test/OpenMP/driver-openmp-amdgpu.c
@@ -0,0 +1,8 @@
+// REQUIRES: x86-registered-target
+// REQUIRES: amdgpu-registered-target
+// REQUIRES: clang-target-64-bits
+
+// Check no warnings about read-only feature emitted.
+// RUN: %clang %s -emit-llvm -S -fopenmp=libomp -nogpulib 
--offload-arch=gfx1030 \
+// RUN:   --offload-device-only -o - 2>&1 | FileCheck --check-prefix=CHECK %s
+// CHECK-NOT: warning: feature flag {{.*}} is ignored since the feature is 
read only
Index: clang/lib/Basic/Targets.cpp
===================================================================
--- clang/lib/Basic/Targets.cpp
+++ 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 @@
 
   // 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;
Index: clang/lib/Basic/TargetInfo.cpp
===================================================================
--- clang/lib/Basic/TargetInfo.cpp
+++ clang/lib/Basic/TargetInfo.cpp
@@ -528,8 +528,6 @@
     // 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] == '+');
   }



Index: clang/test/OpenMP/driver-openmp-amdgpu.c
===================================================================
--- /dev/null
+++ clang/test/OpenMP/driver-openmp-amdgpu.c
@@ -0,0 +1,8 @@
+// REQUIRES: x86-registered-target
+// REQUIRES: amdgpu-registered-target
+// REQUIRES: clang-target-64-bits
+
+// Check no warnings about read-only feature emitted.
+// RUN: %clang %s -emit-llvm -S -fopenmp=libomp -nogpulib --offload-arch=gfx1030 \
+// RUN:   --offload-device-only -o - 2>&1 | FileCheck --check-prefix=CHECK %s
+// CHECK-NOT: warning: feature flag {{.*}} is ignored since the feature is read only
Index: clang/lib/Basic/Targets.cpp
===================================================================
--- clang/lib/Basic/Targets.cpp
+++ 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 @@
 
   // 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;
Index: clang/lib/Basic/TargetInfo.cpp
===================================================================
--- clang/lib/Basic/TargetInfo.cpp
+++ clang/lib/Basic/TargetInfo.cpp
@@ -528,8 +528,6 @@
     // 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] == '+');
   }

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