Author: Yaxun (Sam) Liu Date: 2022-05-19T11:34:42-04:00 New Revision: cefe472c51fbcd1aed4d4a090709f25a12a8bc2c
URL: https://github.com/llvm/llvm-project/commit/cefe472c51fbcd1aed4d4a090709f25a12a8bc2c DIFF: https://github.com/llvm/llvm-project/commit/cefe472c51fbcd1aed4d4a090709f25a12a8bc2c.diff LOG: [clang] Fix __has_builtin Fix __has_builtin to return 1 only if the requested target features of a builtin are enabled by refactoring the code for checking required target features of a builtin and use it in evaluation of __has_builtin. Reviewed by: Artem Belevich Differential Revision: https://reviews.llvm.org/D125829 Added: clang/lib/Basic/BuiltinTargetFeatures.h clang/test/Preprocessor/hash_builtin.cpp Modified: clang/include/clang/Basic/Builtins.h clang/lib/Basic/Builtins.cpp clang/lib/CodeGen/CodeGenFunction.cpp clang/lib/CodeGen/CodeGenFunction.h clang/lib/Lex/PPMacroExpansion.cpp clang/test/Preprocessor/feature_tests.c clang/unittests/CodeGen/CheckTargetFeaturesTest.cpp Removed: ################################################################################ diff --git a/clang/include/clang/Basic/Builtins.h b/clang/include/clang/Basic/Builtins.h index a82e15730f992..f7348f2b2a7c9 100644 --- a/clang/include/clang/Basic/Builtins.h +++ b/clang/include/clang/Basic/Builtins.h @@ -16,6 +16,8 @@ #define LLVM_CLANG_BASIC_BUILTINS_H #include "llvm/ADT/ArrayRef.h" +#include "llvm/ADT/StringMap.h" +#include "llvm/ADT/StringRef.h" #include <cstring> // VC++ defines 'alloca' as an object-like macro, which interferes with our @@ -263,7 +265,15 @@ class Context { const char *Fmt) const; }; -} +/// Returns true if the required target features of a builtin function are +/// enabled. +/// \p TargetFeatureMap maps a target feature to true if it is enabled and +/// false if it is disabled. +bool evaluateRequiredTargetFeatures( + llvm::StringRef RequiredFatures, + const llvm::StringMap<bool> &TargetFetureMap); + +} // namespace Builtin /// Kinds of BuiltinTemplateDecl. enum BuiltinTemplateKind : int { diff --git a/clang/lib/Basic/BuiltinTargetFeatures.h b/clang/lib/Basic/BuiltinTargetFeatures.h new file mode 100644 index 0000000000000..4d3358de50761 --- /dev/null +++ b/clang/lib/Basic/BuiltinTargetFeatures.h @@ -0,0 +1,95 @@ +//===-- CodeGenFunction.h - Target features for builtin ---------*- C++ -*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// +// +// This is the internal required target features for builtin. +// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_CLANG_LIB_BASIC_BUILTINTARGETFEATURES_H +#define LLVM_CLANG_LIB_BASIC_BUILTINTARGETFEATURES_H +#include "llvm/ADT/StringMap.h" +#include "llvm/ADT/StringRef.h" + +using llvm::StringRef; + +namespace clang { +namespace Builtin { +/// TargetFeatures - This class is used to check whether the builtin function +/// has the required tagert specific features. It is able to support the +/// combination of ','(and), '|'(or), and '()'. By default, the priority of +/// ',' is higher than that of '|' . +/// E.g: +/// A,B|C means the builtin function requires both A and B, or C. +/// If we want the builtin function requires both A and B, or both A and C, +/// there are two ways: A,B|A,C or A,(B|C). +/// The FeaturesList should not contain spaces, and brackets must appear in +/// pairs. +class TargetFeatures { + struct FeatureListStatus { + bool HasFeatures; + StringRef CurFeaturesList; + }; + + const llvm::StringMap<bool> &CallerFeatureMap; + + FeatureListStatus getAndFeatures(StringRef FeatureList) { + int InParentheses = 0; + bool HasFeatures = true; + size_t SubexpressionStart = 0; + for (size_t i = 0, e = FeatureList.size(); i < e; ++i) { + char CurrentToken = FeatureList[i]; + switch (CurrentToken) { + default: + break; + case '(': + if (InParentheses == 0) + SubexpressionStart = i + 1; + ++InParentheses; + break; + case ')': + --InParentheses; + assert(InParentheses >= 0 && "Parentheses are not in pair"); + LLVM_FALLTHROUGH; + case '|': + case ',': + if (InParentheses == 0) { + if (HasFeatures && i != SubexpressionStart) { + StringRef F = FeatureList.slice(SubexpressionStart, i); + HasFeatures = CurrentToken == ')' ? hasRequiredFeatures(F) + : CallerFeatureMap.lookup(F); + } + SubexpressionStart = i + 1; + if (CurrentToken == '|') { + return {HasFeatures, FeatureList.substr(SubexpressionStart)}; + } + } + break; + } + } + assert(InParentheses == 0 && "Parentheses are not in pair"); + if (HasFeatures && SubexpressionStart != FeatureList.size()) + HasFeatures = + CallerFeatureMap.lookup(FeatureList.substr(SubexpressionStart)); + return {HasFeatures, StringRef()}; + } + +public: + bool hasRequiredFeatures(StringRef FeatureList) { + FeatureListStatus FS = {false, FeatureList}; + while (!FS.HasFeatures && !FS.CurFeaturesList.empty()) + FS = getAndFeatures(FS.CurFeaturesList); + return FS.HasFeatures; + } + + TargetFeatures(const llvm::StringMap<bool> &CallerFeatureMap) + : CallerFeatureMap(CallerFeatureMap) {} +}; + +} // namespace Builtin +} // namespace clang +#endif /* CLANG_LIB_BASIC_BUILTINTARGETFEATURES_H */ diff --git a/clang/lib/Basic/Builtins.cpp b/clang/lib/Basic/Builtins.cpp index ef8bb562ac17e..b42e8f416cfca 100644 --- a/clang/lib/Basic/Builtins.cpp +++ b/clang/lib/Basic/Builtins.cpp @@ -11,6 +11,7 @@ //===----------------------------------------------------------------------===// #include "clang/Basic/Builtins.h" +#include "BuiltinTargetFeatures.h" #include "clang/Basic/IdentifierTable.h" #include "clang/Basic/LangOptions.h" #include "clang/Basic/TargetInfo.h" @@ -211,3 +212,14 @@ bool Builtin::Context::canBeRedeclared(unsigned ID) const { (!hasReferenceArgsOrResult(ID) && !hasCustomTypechecking(ID)) || isInStdNamespace(ID); } + +bool Builtin::evaluateRequiredTargetFeatures( + StringRef RequiredFeatures, const llvm::StringMap<bool> &TargetFetureMap) { + // Return true if the builtin doesn't have any required features. + if (RequiredFeatures.empty()) + return true; + assert(!RequiredFeatures.contains(' ') && "Space in feature list"); + + TargetFeatures TF(TargetFetureMap); + return TF.hasRequiredFeatures(RequiredFeatures); +} diff --git a/clang/lib/CodeGen/CodeGenFunction.cpp b/clang/lib/CodeGen/CodeGenFunction.cpp index 32d329a69b9f5..7d75d186969ae 100644 --- a/clang/lib/CodeGen/CodeGenFunction.cpp +++ b/clang/lib/CodeGen/CodeGenFunction.cpp @@ -2550,16 +2550,13 @@ void CodeGenFunction::checkTargetFeatures(SourceLocation Loc, llvm::StringMap<bool> CallerFeatureMap; CGM.getContext().getFunctionFeatureMap(CallerFeatureMap, FD); if (BuiltinID) { - StringRef FeatureList( - CGM.getContext().BuiltinInfo.getRequiredFeatures(BuiltinID)); - // Return if the builtin doesn't have any required features. - if (FeatureList.empty()) - return; - assert(!FeatureList.contains(' ') && "Space in feature list"); - TargetFeatures TF(CallerFeatureMap); - if (!TF.hasRequiredFeatures(FeatureList)) + StringRef FeatureList(CGM.getContext().BuiltinInfo.getRequiredFeatures(BuiltinID)); + if (!Builtin::evaluateRequiredTargetFeatures( + FeatureList, CallerFeatureMap)) { CGM.getDiags().Report(Loc, diag::err_builtin_needs_feature) - << TargetDecl->getDeclName() << FeatureList; + << TargetDecl->getDeclName() + << FeatureList; + } } else if (!TargetDecl->isMultiVersion() && TargetDecl->hasAttr<TargetAttr>()) { // Get the required features for the callee. diff --git a/clang/lib/CodeGen/CodeGenFunction.h b/clang/lib/CodeGen/CodeGenFunction.h index b3694f4e2ae2e..938db2a887c59 100644 --- a/clang/lib/CodeGen/CodeGenFunction.h +++ b/clang/lib/CodeGen/CodeGenFunction.h @@ -4814,76 +4814,6 @@ class CodeGenFunction : public CodeGenTypeCache { llvm::Value *FormResolverCondition(const MultiVersionResolverOption &RO); }; -/// TargetFeatures - This class is used to check whether the builtin function -/// has the required tagert specific features. It is able to support the -/// combination of ','(and), '|'(or), and '()'. By default, the priority of -/// ',' is higher than that of '|' . -/// E.g: -/// A,B|C means the builtin function requires both A and B, or C. -/// If we want the builtin function requires both A and B, or both A and C, -/// there are two ways: A,B|A,C or A,(B|C). -/// The FeaturesList should not contain spaces, and brackets must appear in -/// pairs. -class TargetFeatures { - struct FeatureListStatus { - bool HasFeatures; - StringRef CurFeaturesList; - }; - - const llvm::StringMap<bool> &CallerFeatureMap; - - FeatureListStatus getAndFeatures(StringRef FeatureList) { - int InParentheses = 0; - bool HasFeatures = true; - size_t SubexpressionStart = 0; - for (size_t i = 0, e = FeatureList.size(); i < e; ++i) { - char CurrentToken = FeatureList[i]; - switch (CurrentToken) { - default: - break; - case '(': - if (InParentheses == 0) - SubexpressionStart = i + 1; - ++InParentheses; - break; - case ')': - --InParentheses; - assert(InParentheses >= 0 && "Parentheses are not in pair"); - LLVM_FALLTHROUGH; - case '|': - case ',': - if (InParentheses == 0) { - if (HasFeatures && i != SubexpressionStart) { - StringRef F = FeatureList.slice(SubexpressionStart, i); - HasFeatures = CurrentToken == ')' ? hasRequiredFeatures(F) - : CallerFeatureMap.lookup(F); - } - SubexpressionStart = i + 1; - if (CurrentToken == '|') { - return {HasFeatures, FeatureList.substr(SubexpressionStart)}; - } - } - break; - } - } - assert(InParentheses == 0 && "Parentheses are not in pair"); - if (HasFeatures && SubexpressionStart != FeatureList.size()) - HasFeatures = - CallerFeatureMap.lookup(FeatureList.substr(SubexpressionStart)); - return {HasFeatures, StringRef()}; - } - -public: - bool hasRequiredFeatures(StringRef FeatureList) { - FeatureListStatus FS = {false, FeatureList}; - while (!FS.HasFeatures && !FS.CurFeaturesList.empty()) - FS = getAndFeatures(FS.CurFeaturesList); - return FS.HasFeatures; - } - - TargetFeatures(const llvm::StringMap<bool> &CallerFeatureMap) - : CallerFeatureMap(CallerFeatureMap) {} -}; inline DominatingLLVMValue::saved_type DominatingLLVMValue::save(CodeGenFunction &CGF, llvm::Value *value) { diff --git a/clang/lib/Lex/PPMacroExpansion.cpp b/clang/lib/Lex/PPMacroExpansion.cpp index 3109ac44117e0..9d1090be8e096 100644 --- a/clang/lib/Lex/PPMacroExpansion.cpp +++ b/clang/lib/Lex/PPMacroExpansion.cpp @@ -1640,7 +1640,9 @@ void Preprocessor::ExpandBuiltinMacro(Token &Tok) { // usual allocation and deallocation functions. Required by libc++ return 201802; default: - return true; + return Builtin::evaluateRequiredTargetFeatures( + getBuiltinInfo().getRequiredFeatures(II->getBuiltinID()), + getTargetInfo().getTargetOpts().FeatureMap); } return true; } else if (II->getTokenID() != tok::identifier || diff --git a/clang/test/Preprocessor/feature_tests.c b/clang/test/Preprocessor/feature_tests.c index 2568e291b90d0..929ff7d3c498b 100644 --- a/clang/test/Preprocessor/feature_tests.c +++ b/clang/test/Preprocessor/feature_tests.c @@ -1,5 +1,5 @@ -// RUN: %clang_cc1 %s -triple=i686-apple-darwin9 -verify -DVERIFY -// RUN: %clang_cc1 %s -E -triple=i686-apple-darwin9 +// RUN: %clang_cc1 %s -triple=i686-apple-darwin9 -target-cpu pentium4 -verify -DVERIFY +// RUN: %clang_cc1 %s -E -triple=i686-apple-darwin9 -target-cpu pentium4 #ifndef __has_feature #error Should have __has_feature #endif diff --git a/clang/test/Preprocessor/hash_builtin.cpp b/clang/test/Preprocessor/hash_builtin.cpp new file mode 100644 index 0000000000000..77d186c7883f2 --- /dev/null +++ b/clang/test/Preprocessor/hash_builtin.cpp @@ -0,0 +1,11 @@ +// RUN: %clang_cc1 -triple amdgcn -target-cpu gfx906 -E %s -o - | FileCheck %s + +// CHECK: has_s_memtime_inst +#if __has_builtin(__builtin_amdgcn_s_memtime) + int has_s_memtime_inst; +#endif + +// CHECK-NOT: has_gfx10_inst +#if __has_builtin(__builtin_amdgcn_mov_dpp8) + int has_gfx10_inst; +#endif diff --git a/clang/unittests/CodeGen/CheckTargetFeaturesTest.cpp b/clang/unittests/CodeGen/CheckTargetFeaturesTest.cpp index 160b387338d5d..b234ca6bcb0fe 100644 --- a/clang/unittests/CodeGen/CheckTargetFeaturesTest.cpp +++ b/clang/unittests/CodeGen/CheckTargetFeaturesTest.cpp @@ -1,4 +1,4 @@ -#include "../lib/CodeGen/CodeGenFunction.h" +#include "../lib/Basic/BuiltinTargetFeatures.h" #include "gtest/gtest.h" using namespace llvm; @@ -11,7 +11,7 @@ TEST(CheckTargetFeaturesTest, checkBuiltinFeatures) { StringMap<bool> SM; for (StringRef F : Features) SM.insert(std::make_pair(F, true)); - clang::CodeGen::TargetFeatures TF(SM); + clang::Builtin::TargetFeatures TF(SM); return TF.hasRequiredFeatures(BuiltinFeatures); }; // Make sure the basic function ',' and '|' works correctly _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
