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

Yaxun Liu via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri Jun 16 04:55:04 PDT 2023


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] == '+');
   }


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D153123.532094.patch
Type: text/x-patch
Size: 2117 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20230616/f017ac09/attachment.bin>


More information about the cfe-commits mailing list