From c8c583071591dbc6927955e608dcd44c0bac8b9c Mon Sep 17 00:00:00 2001 From: "Yaxun (Sam) Liu" Date: Fri, 16 Jun 2023 00:36:00 -0400 Subject: [PATCH] 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 --- clang/lib/Basic/TargetInfo.cpp | 2 -- clang/lib/Basic/Targets.cpp | 8 ++++++++ clang/test/OpenMP/openmp-read-only-feature.c | 16 ++++++++++++++++ 3 files changed, 24 insertions(+), 2 deletions(-) create mode 100644 clang/test/OpenMP/openmp-read-only-feature.c diff --git a/clang/lib/Basic/TargetInfo.cpp b/clang/lib/Basic/TargetInfo.cpp index 3f0c9d6..6cd5d61 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 ab4bdd0..636b59f 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 0000000..051ffea --- /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 -- 2.7.4