[clang] Fix __has_builtin

[yaxunl]

Summary

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.

[tra]

@echristo - FYI, in case you have an opinion on target builtin feature checks.

clang/include/clang/Basic/Builtins.h

264

I think we should boil it down to just evaluateRequiredTargetFeatures(StringRef RequiredFeatures, ... TargetFeatureMap).

clang/lib/Basic/BuiltinTargetFeatures.h

33

I don't think it needs to be part of the public API. Feature check function operates on llvm::StringMap<bool> and does not need to know about this class, and the implementation of this class can be moved into Builtins.cpp where it's actually used.

[yaxunl]

clang/include/clang/Basic/Builtins.h

264

will do

clang/lib/Basic/BuiltinTargetFeatures.h

33

It is not a public API since it is under lib/Basic.

The reason it is made a header file because we have a unit test clang/unittests/CodeGen/CheckTargetFeaturesTest.cpp for class TargetFeatures. This is because TargetFeatures is relatively complicated.

[yaxunl]

revised by Artem's comments

[tra]

LGTM with a possible nit.

clang/lib/Basic/BuiltinTargetFeatures.h

33

Got it. I've missed the test.

clang/lib/CodeGen/CodeGenFunction.cpp

2552–2553

Should we move that into the evaluateRequiredTargetFeatures ?

Empty feature list -> true sounds like something that belongs to the evaluation logic. So does the assertion on the no-spaces assumption we make about the input.

[yaxunl]

clang/lib/CodeGen/CodeGenFunction.cpp

2552–2553

will do when committing

clang/include/clang/Basic/Builtins.h

 /// functions.
 ///
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_CLANG_BASIC_BUILTINS_H
 #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
 // builtins.
 #undef alloca
 
 namespace clang {
 class TargetInfo;
public:
   /// Returns true if this is a builtin that can be redeclared.  Returns true
   /// for non-builtins.
   bool canBeRedeclared(unsigned ID) const;
 
 private:
   const Info &getRecord(unsigned ID) const;
 
   /// Helper function for isPrintfLike and isScanfLike.
   bool isLike(unsigned ID, unsigned &FormatIdx, bool &HasVAListArg,
               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 {
   /// This names the __make_integer_seq BuiltinTemplateDecl.
   BTK__make_integer_seq,
 
   /// This names the __type_pack_element BuiltinTemplateDecl.
   BTK__type_pack_element
 };
 
 } // end namespace clang
 #endif

clang/lib/Basic/BuiltinTargetFeatures.h

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

clang/lib/Basic/Builtins.cpp

 //===--- Builtins.cpp - Builtin function implementation -------------------===//
 //
 // 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 file implements various things for builtin functions.
 //
 //===----------------------------------------------------------------------===//
 
 #include "clang/Basic/Builtins.h"
+#include "BuiltinTargetFeatures.h"
 #include "clang/Basic/IdentifierTable.h"
 #include "clang/Basic/LangOptions.h"
 #include "clang/Basic/TargetInfo.h"
 #include "llvm/ADT/StringRef.h"
 using namespace clang;
 
 static const Builtin::Info BuiltinInfo[] = {
   { "not a builtin function", nullptr, nullptr, nullptr, ALL_LANGUAGES,nullptr},
bool Builtin::Context::performsCallback(unsigned ID,
   return true;
 }
 
 bool Builtin::Context::canBeRedeclared(unsigned ID) const {
   return ID == Builtin::NotBuiltin || ID == Builtin::BI__va_start ||
          (!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);
+}

clang/lib/CodeGen/CodeGenFunction.h

private:
   llvm::Value *EmitX86CpuIs(StringRef CPUStr);
   llvm::Value *EmitX86CpuSupports(const CallExpr *E);
   llvm::Value *EmitX86CpuSupports(ArrayRef<StringRef> FeatureStrs);
   llvm::Value *EmitX86CpuSupports(uint64_t Mask);
   llvm::Value *EmitX86CpuInit();
   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) {
   if (!needsSaving(value)) return saved_type(value, false);
 
   // Otherwise, we need an alloca.
   auto align = CharUnits::fromQuantity(
             CGF.CGM.getDataLayout().getPrefTypeAlignment(value->getType()));

clang/lib/CodeGen/CodeGenFunction.cpp

void CodeGenFunction::checkTargetFeatures(SourceLocation Loc,
 
   // Grab the required features for the call. For a builtin this is listed in
   // the td file with the default cpu, for an always_inline function this is any
   // listed cpu and any listed features.
   unsigned BuiltinID = TargetDecl->getBuiltinID();
   std::string MissingFeature;
   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.
 
     const TargetAttr *TD = TargetDecl->getAttr<TargetAttr>();
     ParsedTargetAttr ParsedAttr =
         CGM.getContext().filterFunctionTargetAttrs(TD);
 

clang/lib/Lex/PPMacroExpansion.cpp

EvaluateFeatureLikeBuiltinMacro(OS, Tok, II, *this, false,
         else if (II->getBuiltinID() != 0) {
           switch (II->getBuiltinID()) {
           case Builtin::BI__builtin_operator_new:
           case Builtin::BI__builtin_operator_delete:
             // denotes date of behavior change to support calling arbitrary
             // 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 ||
                    II->hasRevertedTokenIDToIdentifier()) {
           // Treat all keywords that introduce a custom syntax of the form
           //
           //   '__some_keyword' '(' [...] ')'
           //

clang/test/Preprocessor/feature_tests.c

-// 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
 
 
 #if __has_feature(something_we_dont_have)
 #error Bad
 #endif

clang/test/Preprocessor/hash_builtin.cpp

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

clang/unittests/CodeGen/CheckTargetFeaturesTest.cpp

-#include "../lib/CodeGen/CodeGenFunction.h"
+#include "../lib/Basic/BuiltinTargetFeatures.h"
 #include "gtest/gtest.h"
 
 using namespace llvm;
 
 // These tests are to Check whether CodeGen::TargetFeatures works correctly.
 TEST(CheckTargetFeaturesTest, checkBuiltinFeatures) {
   auto doCheck = [](StringRef BuiltinFeatures, StringRef FuncFeatures) {
     SmallVector<StringRef, 1> Features;
     FuncFeatures.split(Features, ',');
     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
   ASSERT_FALSE(doCheck("A,B,C,D", "A"));
   ASSERT_TRUE(doCheck("A,B,C,D", "A,B,C,D"));
   ASSERT_TRUE(doCheck("A|B", "A"));
   ASSERT_FALSE(doCheck("A|B", "C"));