[clang] eff08f4 - Revert "[Sema][X86] Consider target attribute into the checks in validateOutputSize and validateInputSize."

Reid Kleckner via cfe-commits cfe-commits at lists.llvm.org
Fri Dec 6 15:42:38 PST 2019


Author: Reid Kleckner
Date: 2019-12-06T15:42:14-08:00
New Revision: eff08f40976e177923fe95759917e59375458f71

URL: https://github.com/llvm/llvm-project/commit/eff08f40976e177923fe95759917e59375458f71
DIFF: https://github.com/llvm/llvm-project/commit/eff08f40976e177923fe95759917e59375458f71.diff

LOG: Revert "[Sema][X86] Consider target attribute into the checks in validateOutputSize and validateInputSize."

This reverts commit e1578fd2b79fe5af5f80c0c166a8abd0f816c022.

It introduces a dependency on Attr.h which I am removing from
ASTContext.h.

Added: 
    

Modified: 
    clang/include/clang/AST/ASTContext.h
    clang/include/clang/Basic/TargetInfo.h
    clang/lib/AST/ASTContext.cpp
    clang/lib/Basic/Targets/X86.cpp
    clang/lib/Basic/Targets/X86.h
    clang/lib/CodeGen/CodeGenFunction.cpp
    clang/lib/CodeGen/CodeGenModule.cpp
    clang/lib/CodeGen/CodeGenModule.h
    clang/lib/Sema/SemaStmtAsm.cpp
    clang/test/CodeGen/x86_32-inline-asm.c

Removed: 
    


################################################################################
diff  --git a/clang/include/clang/AST/ASTContext.h b/clang/include/clang/AST/ASTContext.h
index 6f49dc02be07..024aa1cb021e 100644
--- a/clang/include/clang/AST/ASTContext.h
+++ b/clang/include/clang/AST/ASTContext.h
@@ -95,7 +95,6 @@ class CXXRecordDecl;
 class DiagnosticsEngine;
 class Expr;
 class FixedPointSemantics;
-class GlobalDecl;
 class MangleContext;
 class MangleNumberingContext;
 class MaterializeTemporaryExpr;
@@ -2821,16 +2820,6 @@ class ASTContext : public RefCountedBase<ASTContext> {
   /// PredefinedExpr to cache evaluated results.
   StringLiteral *getPredefinedStringLiteralFromCache(StringRef Key) const;
 
-  /// Parses the target attributes passed in, and returns only the ones that are
-  /// valid feature names.
-  TargetAttr::ParsedTargetAttr
-  filterFunctionTargetAttrs(const TargetAttr *TD) const;
-
-  void getFunctionFeatureMap(llvm::StringMap<bool> &FeatureMap,
-                             const FunctionDecl *) const;
-  void getFunctionFeatureMap(llvm::StringMap<bool> &FeatureMap,
-                             GlobalDecl GD) const;
-
   //===--------------------------------------------------------------------===//
   //                    Statistics
   //===--------------------------------------------------------------------===//

diff  --git a/clang/include/clang/Basic/TargetInfo.h b/clang/include/clang/Basic/TargetInfo.h
index ad863fa1edbd..33cecdadc686 100644
--- a/clang/include/clang/Basic/TargetInfo.h
+++ b/clang/include/clang/Basic/TargetInfo.h
@@ -945,14 +945,12 @@ class TargetInfo : public virtual TransferrableTargetInfo,
   bool validateInputConstraint(MutableArrayRef<ConstraintInfo> OutputConstraints,
                                ConstraintInfo &info) const;
 
-  virtual bool validateOutputSize(const llvm::StringMap<bool> &FeatureMap,
-                                  StringRef /*Constraint*/,
+  virtual bool validateOutputSize(StringRef /*Constraint*/,
                                   unsigned /*Size*/) const {
     return true;
   }
 
-  virtual bool validateInputSize(const llvm::StringMap<bool> &FeatureMap,
-                                 StringRef /*Constraint*/,
+  virtual bool validateInputSize(StringRef /*Constraint*/,
                                  unsigned /*Size*/) const {
     return true;
   }

diff  --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp
index ad61a5a56bc3..4fd7e20dac84 100644
--- a/clang/lib/AST/ASTContext.cpp
+++ b/clang/lib/AST/ASTContext.cpp
@@ -10779,66 +10779,3 @@ QualType ASTContext::getCorrespondingSignedFixedPointType(QualType Ty) const {
     llvm_unreachable("Unexpected unsigned fixed point type");
   }
 }
-
-TargetAttr::ParsedTargetAttr
-ASTContext::filterFunctionTargetAttrs(const TargetAttr *TD) const {
-  assert(TD != nullptr);
-  TargetAttr::ParsedTargetAttr ParsedAttr = TD->parse();
-
-  ParsedAttr.Features.erase(
-      llvm::remove_if(ParsedAttr.Features,
-                      [&](const std::string &Feat) {
-                        return !Target->isValidFeatureName(
-                            StringRef{Feat}.substr(1));
-                      }),
-      ParsedAttr.Features.end());
-  return ParsedAttr;
-}
-
-void ASTContext::getFunctionFeatureMap(llvm::StringMap<bool> &FeatureMap,
-                                       const FunctionDecl *FD) const {
-  if (FD)
-    getFunctionFeatureMap(FeatureMap, GlobalDecl().getWithDecl(FD));
-  else
-    Target->initFeatureMap(FeatureMap, getDiagnostics(),
-                           Target->getTargetOpts().CPU,
-                           Target->getTargetOpts().Features);
-}
-
-// Fills in the supplied string map with the set of target features for the
-// passed in function.
-void ASTContext::getFunctionFeatureMap(llvm::StringMap<bool> &FeatureMap,
-                                       GlobalDecl GD) const {
-  StringRef TargetCPU = Target->getTargetOpts().CPU;
-  const FunctionDecl *FD = GD.getDecl()->getAsFunction();
-  if (const auto *TD = FD->getAttr<TargetAttr>()) {
-    TargetAttr::ParsedTargetAttr ParsedAttr = filterFunctionTargetAttrs(TD);
-
-    // Make a copy of the features as passed on the command line into the
-    // beginning of the additional features from the function to override.
-    ParsedAttr.Features.insert(
-        ParsedAttr.Features.begin(),
-        Target->getTargetOpts().FeaturesAsWritten.begin(),
-        Target->getTargetOpts().FeaturesAsWritten.end());
-
-    if (ParsedAttr.Architecture != "" &&
-        Target->isValidCPUName(ParsedAttr.Architecture))
-      TargetCPU = ParsedAttr.Architecture;
-
-    // Now populate the feature map, first with the TargetCPU which is either
-    // the default or a new one from the target attribute string. Then we'll use
-    // the passed in features (FeaturesAsWritten) along with the new ones from
-    // the attribute.
-    Target->initFeatureMap(FeatureMap, getDiagnostics(), TargetCPU,
-                           ParsedAttr.Features);
-  } else if (const auto *SD = FD->getAttr<CPUSpecificAttr>()) {
-    llvm::SmallVector<StringRef, 32> FeaturesTmp;
-    Target->getCPUSpecificCPUDispatchFeatures(
-        SD->getCPUName(GD.getMultiVersionIndex())->getName(), FeaturesTmp);
-    std::vector<std::string> Features(FeaturesTmp.begin(), FeaturesTmp.end());
-    Target->initFeatureMap(FeatureMap, getDiagnostics(), TargetCPU, Features);
-  } else {
-    Target->initFeatureMap(FeatureMap, getDiagnostics(), TargetCPU,
-                           Target->getTargetOpts().Features);
-  }
-}

diff  --git a/clang/lib/Basic/Targets/X86.cpp b/clang/lib/Basic/Targets/X86.cpp
index d099d3742f0b..51f2006ddbdc 100644
--- a/clang/lib/Basic/Targets/X86.cpp
+++ b/clang/lib/Basic/Targets/X86.cpp
@@ -1731,24 +1731,21 @@ bool X86TargetInfo::validateAsmConstraint(
   }
 }
 
-bool X86TargetInfo::validateOutputSize(const llvm::StringMap<bool> &FeatureMap,
-                                       StringRef Constraint,
+bool X86TargetInfo::validateOutputSize(StringRef Constraint,
                                        unsigned Size) const {
   // Strip off constraint modifiers.
   while (Constraint[0] == '=' || Constraint[0] == '+' || Constraint[0] == '&')
     Constraint = Constraint.substr(1);
 
-  return validateOperandSize(FeatureMap, Constraint, Size);
+  return validateOperandSize(Constraint, Size);
 }
 
-bool X86TargetInfo::validateInputSize(const llvm::StringMap<bool> &FeatureMap,
-                                      StringRef Constraint,
+bool X86TargetInfo::validateInputSize(StringRef Constraint,
                                       unsigned Size) const {
-  return validateOperandSize(FeatureMap, Constraint, Size);
+  return validateOperandSize(Constraint, Size);
 }
 
-bool X86TargetInfo::validateOperandSize(const llvm::StringMap<bool> &FeatureMap,
-                                        StringRef Constraint,
+bool X86TargetInfo::validateOperandSize(StringRef Constraint,
                                         unsigned Size) const {
   switch (Constraint[0]) {
   default:
@@ -1773,7 +1770,7 @@ bool X86TargetInfo::validateOperandSize(const llvm::StringMap<bool> &FeatureMap,
     case 'z':
     case '0':
       // XMM0
-      if (FeatureMap.lookup("sse"))
+      if (SSELevel >= SSE1)
         return Size <= 128U;
       return false;
     case 'i':
@@ -1787,10 +1784,10 @@ bool X86TargetInfo::validateOperandSize(const llvm::StringMap<bool> &FeatureMap,
     LLVM_FALLTHROUGH;
   case 'v':
   case 'x':
-    if (FeatureMap.lookup("avx512f"))
+    if (SSELevel >= AVX512F)
       // 512-bit zmm registers can be used if target supports AVX512F.
       return Size <= 512U;
-    else if (FeatureMap.lookup("avx"))
+    else if (SSELevel >= AVX)
       // 256-bit ymm registers can be used if target supports AVX.
       return Size <= 256U;
     return Size <= 128U;

diff  --git a/clang/lib/Basic/Targets/X86.h b/clang/lib/Basic/Targets/X86.h
index ae3fd240ce25..cad869f71230 100644
--- a/clang/lib/Basic/Targets/X86.h
+++ b/clang/lib/Basic/Targets/X86.h
@@ -177,11 +177,9 @@ class LLVM_LIBRARY_VISIBILITY X86TargetInfo : public TargetInfo {
     return false;
   }
 
-  bool validateOutputSize(const llvm::StringMap<bool> &FeatureMap,
-                          StringRef Constraint, unsigned Size) const override;
+  bool validateOutputSize(StringRef Constraint, unsigned Size) const override;
 
-  bool validateInputSize(const llvm::StringMap<bool> &FeatureMap,
-                         StringRef Constraint, unsigned Size) const override;
+  bool validateInputSize(StringRef Constraint, unsigned Size) const override;
 
   virtual bool
   checkCFProtectionReturnSupported(DiagnosticsEngine &Diags) const override {
@@ -193,8 +191,8 @@ class LLVM_LIBRARY_VISIBILITY X86TargetInfo : public TargetInfo {
     return true;
   };
 
-  virtual bool validateOperandSize(const llvm::StringMap<bool> &FeatureMap,
-                                   StringRef Constraint, unsigned Size) const;
+
+  virtual bool validateOperandSize(StringRef Constraint, unsigned Size) const;
 
   std::string convertConstraint(const char *&Constraint) const override;
   const char *getClobbers() const override {
@@ -370,8 +368,7 @@ class LLVM_LIBRARY_VISIBILITY X86_32TargetInfo : public X86TargetInfo {
     return -1;
   }
 
-  bool validateOperandSize(const llvm::StringMap<bool> &FeatureMap,
-                           StringRef Constraint, unsigned Size) const override {
+  bool validateOperandSize(StringRef Constraint, unsigned Size) const override {
     switch (Constraint[0]) {
     default:
       break;
@@ -389,7 +386,7 @@ class LLVM_LIBRARY_VISIBILITY X86_32TargetInfo : public X86TargetInfo {
       return Size <= 64;
     }
 
-    return X86TargetInfo::validateOperandSize(FeatureMap, Constraint, Size);
+    return X86TargetInfo::validateOperandSize(Constraint, Size);
   }
 
   void setMaxAtomicWidth() override {

diff  --git a/clang/lib/CodeGen/CodeGenFunction.cpp b/clang/lib/CodeGen/CodeGenFunction.cpp
index 5b5eb3de83fc..ffccbe2289d9 100644
--- a/clang/lib/CodeGen/CodeGenFunction.cpp
+++ b/clang/lib/CodeGen/CodeGenFunction.cpp
@@ -2224,7 +2224,7 @@ static bool hasRequiredFeatures(const SmallVectorImpl<StringRef> &ReqFeatures,
   // Now build up the set of caller features and verify that all the required
   // features are there.
   llvm::StringMap<bool> CallerFeatureMap;
-  CGM.getContext().getFunctionFeatureMap(CallerFeatureMap, FD);
+  CGM.getFunctionFeatureMap(CallerFeatureMap, GlobalDecl().getWithDecl(FD));
 
   // If we have at least one of the features in the feature list return
   // true, otherwise return false.
@@ -2286,13 +2286,11 @@ void CodeGenFunction::checkTargetFeatures(SourceLocation Loc,
     // Get the required features for the callee.
 
     const TargetAttr *TD = TargetDecl->getAttr<TargetAttr>();
-    TargetAttr::ParsedTargetAttr ParsedAttr =
-        CGM.getContext().filterFunctionTargetAttrs(TD);
+    TargetAttr::ParsedTargetAttr ParsedAttr = CGM.filterFunctionTargetAttrs(TD);
 
     SmallVector<StringRef, 1> ReqFeatures;
     llvm::StringMap<bool> CalleeFeatureMap;
-    CGM.getContext().getFunctionFeatureMap(CalleeFeatureMap,
-                                           GlobalDecl(TargetDecl));
+    CGM.getFunctionFeatureMap(CalleeFeatureMap, TargetDecl);
 
     for (const auto &F : ParsedAttr.Features) {
       if (F[0] == '+' && CalleeFeatureMap.lookup(F.substr(1)))

diff  --git a/clang/lib/CodeGen/CodeGenModule.cpp b/clang/lib/CodeGen/CodeGenModule.cpp
index 29c294f72125..4959b80faec7 100644
--- a/clang/lib/CodeGen/CodeGenModule.cpp
+++ b/clang/lib/CodeGen/CodeGenModule.cpp
@@ -1667,7 +1667,7 @@ bool CodeGenModule::GetCPUAndFeaturesAttributes(GlobalDecl GD,
   bool AddedAttr = false;
   if (TD || SD) {
     llvm::StringMap<bool> FeatureMap;
-    getContext().getFunctionFeatureMap(FeatureMap, GD);
+    getFunctionFeatureMap(FeatureMap, GD);
 
     // Produce the canonical string for this set of features.
     for (const llvm::StringMap<bool>::value_type &Entry : FeatureMap)
@@ -5858,6 +5858,58 @@ void CodeGenModule::AddVTableTypeMetadata(llvm::GlobalVariable *VTable,
   }
 }
 
+TargetAttr::ParsedTargetAttr CodeGenModule::filterFunctionTargetAttrs(const TargetAttr *TD) {
+  assert(TD != nullptr);
+  TargetAttr::ParsedTargetAttr ParsedAttr = TD->parse();
+
+  ParsedAttr.Features.erase(
+      llvm::remove_if(ParsedAttr.Features,
+                      [&](const std::string &Feat) {
+                        return !Target.isValidFeatureName(
+                            StringRef{Feat}.substr(1));
+                      }),
+      ParsedAttr.Features.end());
+  return ParsedAttr;
+}
+
+
+// Fills in the supplied string map with the set of target features for the
+// passed in function.
+void CodeGenModule::getFunctionFeatureMap(llvm::StringMap<bool> &FeatureMap,
+                                          GlobalDecl GD) {
+  StringRef TargetCPU = Target.getTargetOpts().CPU;
+  const FunctionDecl *FD = GD.getDecl()->getAsFunction();
+  if (const auto *TD = FD->getAttr<TargetAttr>()) {
+    TargetAttr::ParsedTargetAttr ParsedAttr = filterFunctionTargetAttrs(TD);
+
+    // Make a copy of the features as passed on the command line into the
+    // beginning of the additional features from the function to override.
+    ParsedAttr.Features.insert(ParsedAttr.Features.begin(),
+                            Target.getTargetOpts().FeaturesAsWritten.begin(),
+                            Target.getTargetOpts().FeaturesAsWritten.end());
+
+    if (ParsedAttr.Architecture != "" &&
+        Target.isValidCPUName(ParsedAttr.Architecture))
+      TargetCPU = ParsedAttr.Architecture;
+
+    // Now populate the feature map, first with the TargetCPU which is either
+    // the default or a new one from the target attribute string. Then we'll use
+    // the passed in features (FeaturesAsWritten) along with the new ones from
+    // the attribute.
+    Target.initFeatureMap(FeatureMap, getDiags(), TargetCPU,
+                          ParsedAttr.Features);
+  } else if (const auto *SD = FD->getAttr<CPUSpecificAttr>()) {
+    llvm::SmallVector<StringRef, 32> FeaturesTmp;
+    Target.getCPUSpecificCPUDispatchFeatures(
+        SD->getCPUName(GD.getMultiVersionIndex())->getName(), FeaturesTmp);
+    std::vector<std::string> Features(FeaturesTmp.begin(), FeaturesTmp.end());
+    Target.initFeatureMap(FeatureMap, getDiags(), TargetCPU, Features);
+  } else {
+    Target.initFeatureMap(FeatureMap, getDiags(), TargetCPU,
+                          Target.getTargetOpts().Features);
+  }
+}
+
 llvm::SanitizerStatReport &CodeGenModule::getSanStats() {
   if (!SanStats)
     SanStats = std::make_unique<llvm::SanitizerStatReport>(&getModule());

diff  --git a/clang/lib/CodeGen/CodeGenModule.h b/clang/lib/CodeGen/CodeGenModule.h
index 70d4e5f9a95f..33d419a02903 100644
--- a/clang/lib/CodeGen/CodeGenModule.h
+++ b/clang/lib/CodeGen/CodeGenModule.h
@@ -1150,6 +1150,14 @@ class CodeGenModule : public CodeGenTypeCache {
   /// It's up to you to ensure that this is safe.
   void AddDefaultFnAttrs(llvm::Function &F);
 
+  /// Parses the target attributes passed in, and returns only the ones that are
+  /// valid feature names.
+  TargetAttr::ParsedTargetAttr filterFunctionTargetAttrs(const TargetAttr *TD);
+
+  // Fills in the supplied string map with the set of target features for the
+  // passed in function.
+  void getFunctionFeatureMap(llvm::StringMap<bool> &FeatureMap, GlobalDecl GD);
+
   StringRef getMangledName(GlobalDecl GD);
   StringRef getBlockMangledName(GlobalDecl GD, const BlockDecl *BD);
 

diff  --git a/clang/lib/Sema/SemaStmtAsm.cpp b/clang/lib/Sema/SemaStmtAsm.cpp
index 5161b8001918..9b051e02d127 100644
--- a/clang/lib/Sema/SemaStmtAsm.cpp
+++ b/clang/lib/Sema/SemaStmtAsm.cpp
@@ -11,7 +11,6 @@
 //===----------------------------------------------------------------------===//
 
 #include "clang/AST/ExprCXX.h"
-#include "clang/AST/GlobalDecl.h"
 #include "clang/AST/RecordLayout.h"
 #include "clang/AST/TypeLoc.h"
 #include "clang/Basic/TargetInfo.h"
@@ -256,10 +255,6 @@ StmtResult Sema::ActOnGCCAsmStmt(SourceLocation AsmLoc, bool IsSimple,
   // The parser verifies that there is a string literal here.
   assert(AsmString->isAscii());
 
-  FunctionDecl *FD = dyn_cast<FunctionDecl>(getCurLexicalContext());
-  llvm::StringMap<bool> FeatureMap;
-  Context.getFunctionFeatureMap(FeatureMap, FD);
-
   for (unsigned i = 0; i != NumOutputs; i++) {
     StringLiteral *Literal = Constraints[i];
     assert(Literal->isAscii());
@@ -330,8 +325,8 @@ StmtResult Sema::ActOnGCCAsmStmt(SourceLocation AsmLoc, bool IsSimple,
     }
 
     unsigned Size = Context.getTypeSize(OutputExpr->getType());
-    if (!Context.getTargetInfo().validateOutputSize(
-            FeatureMap, Literal->getString(), Size)) {
+    if (!Context.getTargetInfo().validateOutputSize(Literal->getString(),
+                                                    Size)) {
       targetDiag(OutputExpr->getBeginLoc(), diag::err_asm_invalid_output_size)
           << Info.getConstraintStr();
       return new (Context)
@@ -432,8 +427,8 @@ StmtResult Sema::ActOnGCCAsmStmt(SourceLocation AsmLoc, bool IsSimple,
         return StmtError();
 
     unsigned Size = Context.getTypeSize(Ty);
-    if (!Context.getTargetInfo().validateInputSize(FeatureMap,
-                                                   Literal->getString(), Size))
+    if (!Context.getTargetInfo().validateInputSize(Literal->getString(),
+                                                   Size))
       return StmtResult(
           targetDiag(InputExpr->getBeginLoc(), diag::err_asm_invalid_input_size)
           << Info.getConstraintStr());

diff  --git a/clang/test/CodeGen/x86_32-inline-asm.c b/clang/test/CodeGen/x86_32-inline-asm.c
index 5a69064abc03..c1fba0eee942 100644
--- a/clang/test/CodeGen/x86_32-inline-asm.c
+++ b/clang/test/CodeGen/x86_32-inline-asm.c
@@ -70,35 +70,3 @@ int func1() {
   __asm__ volatile("foo1 %0" : "=x" (val256)); // expected-error {{invalid output size for constraint '=x'}}
 #endif
 }
-
-int __attribute__((__target__("sse"))) _func2() {
-  __asm__ volatile("foo1 %0" : : "x" (val128)); // No error.
-  __asm__ volatile("foo1 %0" : "=x" (val128));  // No error.
-#ifdef __AVX__
-  __asm__ volatile("foo1 %0" : : "x" (val256)); // No error.
-  __asm__ volatile("foo1 %0" : "=x" (val256));  // No error.
-#else
-  __asm__ volatile("foo1 %0" : : "x" (val256)); // expected-error {{invalid input size for constraint 'x'}}
-  __asm__ volatile("foo1 %0" : "=x" (val256)); // expected-error {{invalid output size for constraint '=x'}}
-#endif
-  __asm__ volatile("foo1 %0" : : "x" (val512)); // expected-error {{invalid input size for constraint 'x'}}
-  __asm__ volatile("foo1 %0" : "=x" (val512)); // expected-error {{invalid output size for constraint '=x'}}
-}
-
-int __attribute__((__target__("avx"))) _func3() {
-  __asm__ volatile("foo1 %0" : : "x" (val128)); // No error.
-  __asm__ volatile("foo1 %0" : "=x" (val128));  // No error.
-  __asm__ volatile("foo1 %0" : : "x" (val256)); // No error.
-  __asm__ volatile("foo1 %0" : "=x" (val256));  // No error.
-  __asm__ volatile("foo1 %0" : : "x" (val512)); // expected-error {{invalid input size for constraint 'x'}}
-  __asm__ volatile("foo1 %0" : "=x" (val512)); // expected-error {{invalid output size for constraint '=x'}}
-}
-
-int __attribute__((__target__("avx512f"))) _func4() {
-  __asm__ volatile("foo1 %0" : : "x" (val128)); // No error.
-  __asm__ volatile("foo1 %0" : "=x" (val128));  // No error.
-  __asm__ volatile("foo1 %0" : : "x" (val256)); // No error.
-  __asm__ volatile("foo1 %0" : "=x" (val256));  // No error.
-  __asm__ volatile("foo1 %0" : : "x" (val512)); // No error.
-  __asm__ volatile("foo1 %0" : "=x" (val512)); // No error.
-}


        


More information about the cfe-commits mailing list