[CodeGen]Refactor CpuSupports/CPUIs Builtin Code Gen to better work with "target" implementation

[erichkeane]

Summary

A small set of refactors that'll make it easier for me to implement 'target' support.

First, extract the CPUSupports functionality into its own function. THis has the advantage of not wasting time in this builtin to deal with arguments.
Second, pulls both CPUSupports and CPUIs implementation into a member-function, so that it can be called from the resolver generation that I'm working on.
Third, creates an overload that takes simply the feature/cpu name (rather than extracting it from a callexpr), since that info isn't available later.

Note that despite how the 'diff' looks, the EmitX86CPUSupports function simply takes the implementation out of the 'switch'.

[erichkeane]

Thinking about it further, the emit functions should be private in this case.

[erichkeane]

Of course, overnight I realized I am probably better off having the 'CpuSupports' validate MULTIPLE features at a time! I'll update this patch with an improvement when I get one that I think is really useful. Sorry for the thrash.

[craig.topper]

LGTM

[erichkeane]

I actually updated it a bit since then, so I'll hold off on committing until my replacement version is perfect, I'd prefer to stop churning. Thanks for the review though Craig!

[erichkeane]

I figured out that combining the CpuSupports items via an array is a REALLY useful function for dispatch. I've got most of gcc's target implemented, so I believe that this is sufficient for that purpose.

Thanks in advance for the reviews :)

[erichkeane]

Woops, messed up my rebase!

[craig.topper]

lib/CodeGen/CGBuiltin.cpp

7414

You shouldn't need curly braces here. ArrayRef has a conversion constructor that should take care of this.

7456

Declare as uint32_t?

7494

No need for 64-bit OR here. FeaturesMask is only 32-bits.

7525

I think you have the builtin handline at the top and in the switch. Bad rebase?

[craig.topper]

LGTM

lib/CodeGen/CGBuiltin.cpp

 
 static Value *EmitX86SExtMask(CodeGenFunction &CGF, Value *Op, 
                               llvm::Type *DstTy) {
   unsigned NumberOfElements = DstTy->getVectorNumElements();
   Value *Mask = getMaskVecValue(CGF, Op, NumberOfElements);
   return CGF.Builder.CreateSExt(Mask, DstTy, "vpmovm2");
 }
 
-static Value *EmitX86CpuIs(CodeGenFunction &CGF, const CallExpr *E) {
+Value *CodeGenFunction::EmitX86CpuIs(const CallExpr *E) {
   const Expr *CPUExpr = E->getArg(0)->IgnoreParenCasts();
   StringRef CPUStr = cast<clang::StringLiteral>(CPUExpr)->getString();
+  return EmitX86CpuIs(CPUStr);
+}
+
+Value *CodeGenFunction::EmitX86CpuIs(StringRef CPUStr) {
 
   // This enum contains the vendor, type, and subtype enums from the
   // runtime library concatenated together. The _START labels mark
   // the start and are used to adjust the value into the correct
   // encoding space.
   enum X86CPUs {
     INTEL = 1,
     AMD,
enum X86CPUs {
     INTEL_COREI7_SKYLAKE,
     INTEL_COREI7_SKYLAKE_AVX512,
   };
 
   X86CPUs CPU =
     StringSwitch<X86CPUs>(CPUStr)
       .Case("amd", AMD)
       .Case("amdfam10h", AMDFAM10H)
+      .Case("amdfam10", AMDFAM10H)
       .Case("amdfam15h", AMDFAM15H)
+      .Case("amdfam15", AMDFAM15H)
       .Case("atom", INTEL_BONNELL)
       .Case("barcelona", AMDFAM10H_BARCELONA)
       .Case("bdver1", AMDFAM15H_BDVER1)
       .Case("bdver2", AMDFAM15H_BDVER2)
       .Case("bdver3", AMDFAM15H_BDVER3)
       .Case("bdver4", AMDFAM15H_BDVER4)
       .Case("bonnell", INTEL_BONNELL)
       .Case("broadwell", INTEL_COREI7_BROADWELL)
StringSwitch<X86CPUs>(CPUStr)
       .Case("shanghai", AMDFAM10H_SHANGHAI)
       .Case("silvermont", INTEL_SILVERMONT)
       .Case("skylake", INTEL_COREI7_SKYLAKE)
       .Case("skylake-avx512", INTEL_COREI7_SKYLAKE_AVX512)
       .Case("slm", INTEL_SILVERMONT)
       .Case("westmere", INTEL_COREI7_WESTMERE)
       .Case("znver1", AMDFAM17H_ZNVER1);
 
-  llvm::Type *Int32Ty = CGF.Builder.getInt32Ty();
+  llvm::Type *Int32Ty = Builder.getInt32Ty();
 
   // Matching the struct layout from the compiler-rt/libgcc structure that is
   // filled in:
   // unsigned int __cpu_vendor;
   // unsigned int __cpu_type;
   // unsigned int __cpu_subtype;
   // unsigned int __cpu_features[1];
   llvm::Type *STy = llvm::StructType::get(Int32Ty, Int32Ty, Int32Ty,
                                           llvm::ArrayType::get(Int32Ty, 1));
 
   // Grab the global __cpu_model.
-  llvm::Constant *CpuModel = CGF.CGM.CreateRuntimeVariable(STy, "__cpu_model");
+  llvm::Constant *CpuModel = CGM.CreateRuntimeVariable(STy, "__cpu_model");
 
   // Calculate the index needed to access the correct field based on the
   // range. Also adjust the expected value.
   unsigned Index;
   unsigned Value;
   if (CPU > CPU_SUBTYPE_START) {
     Index = 2;
     Value = CPU - CPU_SUBTYPE_START;
   } else if (CPU > CPU_TYPE_START) {
     Index = 1;
     Value = CPU - CPU_TYPE_START;
   } else {
     Index = 0;
     Value = CPU;
   }
 
   // Grab the appropriate field from __cpu_model.
   llvm::Value *Idxs[] = {
     ConstantInt::get(Int32Ty, 0),
     ConstantInt::get(Int32Ty, Index)
   };
-  llvm::Value *CpuValue = CGF.Builder.CreateGEP(STy, CpuModel, Idxs);
-  CpuValue = CGF.Builder.CreateAlignedLoad(CpuValue, CharUnits::fromQuantity(4));
+  llvm::Value *CpuValue = Builder.CreateGEP(STy, CpuModel, Idxs);
+  CpuValue = Builder.CreateAlignedLoad(CpuValue, CharUnits::fromQuantity(4));
 
   // Check the value of the field against the requested value.
-  return CGF.Builder.CreateICmpEQ(CpuValue,
+  return Builder.CreateICmpEQ(CpuValue,
                                   llvm::ConstantInt::get(Int32Ty, Value));
 }
 
-Value *CodeGenFunction::EmitX86BuiltinExpr(unsigned BuiltinID,
-                                           const CallExpr *E) {
-  SmallVector<Value*, 4> Ops;
-
-  // Find out if any arguments are required to be integer constant expressions.
-  unsigned ICEArguments = 0;
-  ASTContext::GetBuiltinTypeError Error;
-  getContext().GetBuiltinType(BuiltinID, Error, &ICEArguments);
-  assert(Error == ASTContext::GE_None && "Should not codegen an error");
-
-  for (unsigned i = 0, e = E->getNumArgs(); i != e; i++) {
-    // If this is a normal argument, just emit it as a scalar.
-    if ((ICEArguments & (1 << i)) == 0) {
-      Ops.push_back(EmitScalarExpr(E->getArg(i)));
-      continue;
-    }
-
-    // If this is required to be a constant, constant fold it so that we know
-    // that the generated intrinsic gets a ConstantInt.
-    llvm::APSInt Result;
-    bool IsConst = E->getArg(i)->isIntegerConstantExpr(Result, getContext());
-    assert(IsConst && "Constant arg isn't actually constant?"); (void)IsConst;
-    Ops.push_back(llvm::ConstantInt::get(getLLVMContext(), Result));
-  }
-
-  // These exist so that the builtin that takes an immediate can be bounds
-  // checked by clang to avoid passing bad immediates to the backend. Since
-  // AVX has a larger immediate than SSE we would need separate builtins to
-  // do the different bounds checking. Rather than create a clang specific
-  // SSE only builtin, this implements eight separate builtins to match gcc
-  // implementation.
-  auto getCmpIntrinsicCall = [this, &Ops](Intrinsic::ID ID, unsigned Imm) {
-    Ops.push_back(llvm::ConstantInt::get(Int8Ty, Imm));
-    llvm::Function *F = CGM.getIntrinsic(ID);
-    return Builder.CreateCall(F, Ops);
-  };
-
-  // For the vector forms of FP comparisons, translate the builtins directly to
-  // IR.
-  // TODO: The builtins could be removed if the SSE header files used vector
-  // extension comparisons directly (vector ordered/unordered may need
-  // additional support via __builtin_isnan()).
-  auto getVectorFCmpIR = [this, &Ops](CmpInst::Predicate Pred) {
-    Value *Cmp = Builder.CreateFCmp(Pred, Ops[0], Ops[1]);
-    llvm::VectorType *FPVecTy = cast<llvm::VectorType>(Ops[0]->getType());
-    llvm::VectorType *IntVecTy = llvm::VectorType::getInteger(FPVecTy);
-    Value *Sext = Builder.CreateSExt(Cmp, IntVecTy);
-    return Builder.CreateBitCast(Sext, FPVecTy);
-  };
-
-  switch (BuiltinID) {
-  default: return nullptr;
-  case X86::BI__builtin_cpu_init: {
-    llvm::FunctionType *FTy = llvm::FunctionType::get(VoidTy,
-                                                      /*Variadic*/false);
-    llvm::Constant *Func = CGM.CreateRuntimeFunction(FTy,
-                                                     "__cpu_indicator_init");
-    return Builder.CreateCall(Func);
-  }
-  case X86::BI__builtin_cpu_is:
-    return EmitX86CpuIs(*this, E);
-  case X86::BI__builtin_cpu_supports: {
-    const Expr *FeatureExpr = E->getArg(0)->IgnoreParenCasts();
-    StringRef FeatureStr = cast<StringLiteral>(FeatureExpr)->getString();
+Value *CodeGenFunction::EmitX86CpuSupports(const CallExpr *E) {
+  return EmitX86CpuSupports(FeatureStr);
+}
 
-    // TODO: When/if this becomes more than x86 specific then use a TargetInfo
-    // based mapping.
-    // Processor features and mapping to processor feature value.
-    enum X86Features {
-      CMOV = 0,
-      MMX,
-      POPCNT,
-      SSE,
-      SSE2,
-      SSE3,
-      SSSE3,
-      SSE4_1,
-      SSE4_2,
-      AVX,
-      AVX2,
-      SSE4_A,
-      FMA4,
-      XOP,
-      FMA,
-      AVX512F,
-      BMI,
-      BMI2,
-      AES,
-      PCLMUL,
-      AVX512VL,
-      AVX512BW,
-      AVX512DQ,
-      AVX512CD,
-      AVX512ER,
-      AVX512PF,
-      AVX512VBMI,
-      AVX512IFMA,
-      AVX5124VNNIW,
-      AVX5124FMAPS,
-      AVX512VPOPCNTDQ,
-      MAX
-    };
+Value *CodeGenFunction::EmitX86CpuSupports(ArrayRef<StringRef> FeatureStrs) {
 
+  uint32_t FeaturesMask = 0;
+
+  for (const StringRef &FeatureStr : FeatureStrs) {
     X86Features Feature =
         StringSwitch<X86Features>(FeatureStr)
             .Case("cmov", X86Features::CMOV)
             .Case("mmx", X86Features::MMX)
             .Case("popcnt", X86Features::POPCNT)
             .Case("sse", X86Features::SSE)
             .Case("sse2", X86Features::SSE2)
             .Case("sse3", X86Features::SSE3)
X86Features Feature =
             .Case("avx512pf", X86Features::AVX512PF)
             .Case("avx512vbmi", X86Features::AVX512VBMI)
             .Case("avx512ifma", X86Features::AVX512IFMA)
             .Case("avx5124vnniw", X86Features::AVX5124VNNIW)
             .Case("avx5124fmaps", X86Features::AVX5124FMAPS)
             .Case("avx512vpopcntdq", X86Features::AVX512VPOPCNTDQ)
             .Default(X86Features::MAX);
     assert(Feature != X86Features::MAX && "Invalid feature!");
+    FeaturesMask |= (1U << Feature);
+  }
 
-    // Matching the struct layout from the compiler-rt/libgcc structure that is
-    // filled in:
-    // unsigned int __cpu_vendor;
-    // unsigned int __cpu_type;
-    // unsigned int __cpu_subtype;
-    // unsigned int __cpu_features[1];
-    llvm::Type *STy = llvm::StructType::get(Int32Ty, Int32Ty, Int32Ty,
-                                            llvm::ArrayType::get(Int32Ty, 1));
 
-    // Grab the global __cpu_model.
-    llvm::Constant *CpuModel = CGM.CreateRuntimeVariable(STy, "__cpu_model");
 
-    // Grab the first (0th) element from the field __cpu_features off of the
-    // global in the struct STy.
-    Value *Idxs[] = {
-      ConstantInt::get(Int32Ty, 0),
-      ConstantInt::get(Int32Ty, 3),
-      ConstantInt::get(Int32Ty, 0)
-    };
-    Value *CpuFeatures = Builder.CreateGEP(STy, CpuModel, Idxs);
-    Value *Features = Builder.CreateAlignedLoad(CpuFeatures,
-                                                CharUnits::fromQuantity(4));
+  Value *Idxs[] = {ConstantInt::get(Int32Ty, 0), ConstantInt::get(Int32Ty, 3),
+                   ConstantInt::get(Int32Ty, 0)};
+  Value *Features =
+      Builder.CreateAlignedLoad(CpuFeatures, CharUnits::fromQuantity(4));
 
-    // Check the value of the bit corresponding to the feature requested.
-    Value *Bitset = Builder.CreateAnd(
-        Features, llvm::ConstantInt::get(Int32Ty, 1ULL << Feature));
-    return Builder.CreateICmpNE(Bitset, llvm::ConstantInt::get(Int32Ty, 0));
-  }
+      Features, llvm::ConstantInt::get(Int32Ty, FeaturesMask));
+
+Value *CodeGenFunction::EmitX86BuiltinExpr(unsigned BuiltinID,
+                                           const CallExpr *E) {
+  if (BuiltinID == X86::BI__builtin_cpu_is)
+    return EmitX86CpuIs(E);
+  if (BuiltinID == X86::BI__builtin_cpu_supports)
+    return EmitX86CpuSupports(E);
+
+  SmallVector<Value*, 4> Ops;
+
+  // Find out if any arguments are required to be integer constant expressions.
+  unsigned ICEArguments = 0;
+  ASTContext::GetBuiltinTypeError Error;
+  getContext().GetBuiltinType(BuiltinID, Error, &ICEArguments);
+  assert(Error == ASTContext::GE_None && "Should not codegen an error");
+
+  for (unsigned i = 0, e = E->getNumArgs(); i != e; i++) {
+    // If this is a normal argument, just emit it as a scalar.
+    if ((ICEArguments & (1 << i)) == 0) {
+      Ops.push_back(EmitScalarExpr(E->getArg(i)));
+      continue;
+    }
+
+    // If this is required to be a constant, constant fold it so that we know
+    // that the generated intrinsic gets a ConstantInt.
+    llvm::APSInt Result;
+    bool IsConst = E->getArg(i)->isIntegerConstantExpr(Result, getContext());
+    assert(IsConst && "Constant arg isn't actually constant?"); (void)IsConst;
+    Ops.push_back(llvm::ConstantInt::get(getLLVMContext(), Result));
+  }
+
+  // These exist so that the builtin that takes an immediate can be bounds
+  // checked by clang to avoid passing bad immediates to the backend. Since
+  // AVX has a larger immediate than SSE we would need separate builtins to
+  // do the different bounds checking. Rather than create a clang specific
+  // SSE only builtin, this implements eight separate builtins to match gcc
+  // implementation.
+  auto getCmpIntrinsicCall = [this, &Ops](Intrinsic::ID ID, unsigned Imm) {
+    Ops.push_back(llvm::ConstantInt::get(Int8Ty, Imm));
+    llvm::Function *F = CGM.getIntrinsic(ID);
+    return Builder.CreateCall(F, Ops);
+  };
+
+  // For the vector forms of FP comparisons, translate the builtins directly to
+  // IR.
+  // TODO: The builtins could be removed if the SSE header files used vector
+  // extension comparisons directly (vector ordered/unordered may need
+  // additional support via __builtin_isnan()).
+  auto getVectorFCmpIR = [this, &Ops](CmpInst::Predicate Pred) {
+    Value *Cmp = Builder.CreateFCmp(Pred, Ops[0], Ops[1]);
+    llvm::VectorType *FPVecTy = cast<llvm::VectorType>(Ops[0]->getType());
+    llvm::VectorType *IntVecTy = llvm::VectorType::getInteger(FPVecTy);
+    Value *Sext = Builder.CreateSExt(Cmp, IntVecTy);
+    return Builder.CreateBitCast(Sext, FPVecTy);
+  };
+
+  switch (BuiltinID) {
+  default: return nullptr;
+  case X86::BI__builtin_cpu_init: {
+    llvm::FunctionType *FTy = llvm::FunctionType::get(VoidTy,
+                                                      /*Variadic*/false);
+    llvm::Constant *Func = CGM.CreateRuntimeFunction(FTy,
+                                                     "__cpu_indicator_init");
+    return Builder.CreateCall(Func);
+  }
   case X86::BI_mm_prefetch: {
     Value *Address = Ops[0];
     Value *RW = ConstantInt::get(Int32Ty, 0);
     Value *Locality = Ops[1];
     Value *Data = ConstantInt::get(Int32Ty, 1);
     Value *F = CGM.getIntrinsic(Intrinsic::prefetch);
     return Builder.CreateCall(F, {Address, RW, Locality, Data});
   }

lib/CodeGen/CodeGenFunction.h

private:
   void EmitDeclMetadata();
 
   BlockByrefHelpers *buildByrefHelpers(llvm::StructType &byrefType,
                                   const AutoVarEmission &emission);
 
   void AddObjCARCExceptionMetadata(llvm::Instruction *Inst);
 
   llvm::Value *GetValueForARMHint(unsigned BuiltinID);
+  llvm::Value *EmitX86CpuIs(const CallExpr *E);
+  llvm::Value *EmitX86CpuIs(StringRef CPUStr);
+  llvm::Value *EmitX86CpuSupports(const CallExpr *E);
+  llvm::Value *EmitX86CpuSupports(ArrayRef<StringRef> FeatureStrs);
 };
 
 /// Helper class with most of the code for saving a value for a
 /// conditional expression cleanup.
 struct DominatingLLVMValue {
   typedef llvm::PointerIntPair<llvm::Value*, 1, bool> saved_type;
 
   /// Answer whether the given value needs extra work to be saved.