[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

Author: yxsamliu

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 {

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

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

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.

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) {

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

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

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

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