r250473 - Add an error when calling a builtin that requires features that don't

Eric Christopher via cfe-commits cfe-commits at lists.llvm.org
Fri Oct 16 20:04:14 PDT 2015


Can't be. We don't know we're going to make the call until we code gen. :)

On Fri, Oct 16, 2015, 7:50 PM Justin Bogner <mail at justinbogner.com> wrote:

> Eric Christopher via cfe-commits <cfe-commits at lists.llvm.org> writes:
> > Author: echristo
> > Date: Thu Oct 15 18:47:11 2015
> > New Revision: 250473
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=250473&view=rev
> > Log:
> > Add an error when calling a builtin that requires features that don't
> > match the feature set of the function that they're being called from.
> >
> > This ensures that we can effectively diagnose some[1] code that would
> > instead ICE in the backend with a failure to select message.
> >
> > Example:
> >
> > __m128d foo(__m128d a, __m128d b) {
> >   return __builtin_ia32_addsubps(b, a);
> > }
> >
> > compiled for normal x86_64 via:
> >
> > clang -target x86_64-linux-gnu -c
> >
> > would fail to compile in the back end because the normal subtarget
> > features for x86_64 only include sse2 and the builtin requires sse3.
> >
> > [1] We're still not erroring on:
> >
> > __m128i bar(__m128i const *p) { return _mm_lddqu_si128(p); }
> >
> > where we should fail and error on an always_inline function being
> > inlined into a function that doesn't support the subtarget features
> > required.
> >
> > Added:
> >     cfe/trunk/test/CodeGen/target-builtin-error-2.c
> >     cfe/trunk/test/CodeGen/target-builtin-error.c
> >     cfe/trunk/test/CodeGen/target-builtin-noerror.c
> > Modified:
> >     cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
> >     cfe/trunk/lib/CodeGen/CGBuiltin.cpp
> >     cfe/trunk/lib/CodeGen/CGCall.cpp
> >     cfe/trunk/lib/CodeGen/CodeGenFunction.h
> >
> > Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
> > URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=250473&r1=250472&r2=250473&view=diff
> >
> ==============================================================================
> > --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
> > +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Thu Oct 15
> 18:47:11 2015
> > @@ -430,6 +430,7 @@ def warn_redecl_library_builtin : Warnin
> >  def err_builtin_definition : Error<"definition of builtin function %0">;
> >  def err_arm_invalid_specialreg : Error<"invalid special register for
> builtin">;
> >  def err_invalid_cpu_supports : Error<"invalid cpu feature string for
> builtin">;
> > +def err_builtin_needs_feature : Error<"%0 needs target feature %1">;
> >  def warn_builtin_unknown : Warning<"use of unknown builtin %0">,
> >    InGroup<ImplicitFunctionDeclare>, DefaultError;
> >  def warn_dyn_class_memaccess : Warning<
> >
> > Modified: cfe/trunk/lib/CodeGen/CGBuiltin.cpp
> > URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGBuiltin.cpp?rev=250473&r1=250472&r2=250473&view=diff
> >
> ==============================================================================
> > --- cfe/trunk/lib/CodeGen/CGBuiltin.cpp (original)
> > +++ cfe/trunk/lib/CodeGen/CGBuiltin.cpp Thu Oct 15 18:47:11 2015
> > @@ -21,6 +21,7 @@
> >  #include "clang/Basic/TargetBuiltins.h"
> >  #include "clang/Basic/TargetInfo.h"
> >  #include "clang/CodeGen/CGFunctionInfo.h"
> > +#include "clang/Sema/SemaDiagnostic.h"
> >  #include "llvm/ADT/StringExtras.h"
> >  #include "llvm/IR/CallSite.h"
> >  #include "llvm/IR/DataLayout.h"
> > @@ -288,6 +289,62 @@ Value *CodeGenFunction::EmitVAStartEnd(V
> >    return Builder.CreateCall(CGM.getIntrinsic(inst), ArgValue);
> >  }
> >
> > +// Returns true if we have a valid set of target features.
> > +bool CodeGenFunction::checkBuiltinTargetFeatures(
> > +    const FunctionDecl *TargetDecl) {
> > +  // Early exit if this is an indirect call.
> > +  if (!TargetDecl)
> > +    return true;
> > +
> > +  // Get the current enclosing function if it exists.
> > +  if (const FunctionDecl *FD =
> dyn_cast_or_null<FunctionDecl>(CurFuncDecl)) {
> > +    unsigned BuiltinID = TargetDecl->getBuiltinID();
> > +    const char *FeatureList =
> > +        CGM.getContext().BuiltinInfo.getRequiredFeatures(BuiltinID);
> > +    if (FeatureList && StringRef(FeatureList) != "") {
> > +      StringRef TargetCPU = Target.getTargetOpts().CPU;
> > +      llvm::StringMap<bool> FeatureMap;
> > +
> > +      if (const auto *TD = FD->getAttr<TargetAttr>()) {
> > +        // If we have a TargetAttr build up the feature map based on
> that.
> > +        TargetAttr::ParsedTargetAttr ParsedAttr = TD->parse();
> > +
> > +        // 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.first.insert(
> > +            ParsedAttr.first.begin(),
> > +            Target.getTargetOpts().FeaturesAsWritten.begin(),
> > +            Target.getTargetOpts().FeaturesAsWritten.end());
> > +
> > +        if (ParsedAttr.second != "")
> > +          TargetCPU = ParsedAttr.second;
> > +
> > +        // 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, CGM.getDiags(), TargetCPU,
> > +                              ParsedAttr.first);
> > +      } else {
> > +        Target.initFeatureMap(FeatureMap, CGM.getDiags(), TargetCPU,
> > +                              Target.getTargetOpts().Features);
> > +      }
> > +
> > +      // If we have at least one of the features in the feature list
> return
> > +      // true, otherwise return false.
> > +      SmallVector<StringRef, 1> AttrFeatures;
> > +      StringRef(FeatureList).split(AttrFeatures, ",");
> > +      for (const auto &Feature : AttrFeatures)
> > +        if (FeatureMap[Feature])
> > +       return true;
> > +      return false;
> > +    }
> > +  }
> > +  return true;
> > +}
> > +
> >  RValue CodeGenFunction::EmitBuiltinExpr(const FunctionDecl *FD,
> >                                          unsigned BuiltinID, const
> CallExpr *E,
> >                                          ReturnValueSlot ReturnValue) {
> > @@ -1789,6 +1846,16 @@ RValue CodeGenFunction::EmitBuiltinExpr(
> >    if (getContext().BuiltinInfo.isPredefinedLibFunction(BuiltinID))
> >      return emitLibraryCall(*this, FD, E,
> EmitScalarExpr(E->getCallee()));
> >
> > +  // Check that a call to a target specific builtin has the correct
> target
> > +  // features.
> > +  // This is down here to avoid non-target specific builtins, however,
> if
> > +  // generic builtins start to require generic target features then we
> > +  // can move this up to the beginning of the function.
> > +  if (!checkBuiltinTargetFeatures(FD))
> > +    CGM.getDiags().Report(E->getLocStart(),
> diag::err_builtin_needs_feature)
> > +        << FD->getDeclName()
> > +        << CGM.getContext().BuiltinInfo.getRequiredFeatures(BuiltinID);
>
> This check really belongs in Sema, not CodeGen. I'd expect it to be part
> of CheckBuiltinFunctionCall in SemaChecking.cpp.
>
> > +
> >    // See if we have a target specific intrinsic.
> >    const char *Name = getContext().BuiltinInfo.getName(BuiltinID);
> >    Intrinsic::ID IntrinsicID = Intrinsic::not_intrinsic;
> >
> > Modified: cfe/trunk/lib/CodeGen/CGCall.cpp
> > URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGCall.cpp?rev=250473&r1=250472&r2=250473&view=diff
> >
> ==============================================================================
> > --- cfe/trunk/lib/CodeGen/CGCall.cpp (original)
> > +++ cfe/trunk/lib/CodeGen/CGCall.cpp Thu Oct 15 18:47:11 2015
> > @@ -21,6 +21,7 @@
> >  #include "clang/AST/Decl.h"
> >  #include "clang/AST/DeclCXX.h"
> >  #include "clang/AST/DeclObjC.h"
> > +#include "clang/Basic/TargetBuiltins.h"
> >  #include "clang/Basic/TargetInfo.h"
> >  #include "clang/CodeGen/CGFunctionInfo.h"
> >  #include "clang/Frontend/CodeGenOptions.h"
> >
> > Modified: cfe/trunk/lib/CodeGen/CodeGenFunction.h
> > URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenFunction.h?rev=250473&r1=250472&r2=250473&view=diff
> >
> ==============================================================================
> > --- cfe/trunk/lib/CodeGen/CodeGenFunction.h (original)
> > +++ cfe/trunk/lib/CodeGen/CodeGenFunction.h Thu Oct 15 18:47:11 2015
> > @@ -2633,6 +2633,8 @@ public:
> >    RValue EmitCallExpr(const CallExpr *E,
> >                        ReturnValueSlot ReturnValue = ReturnValueSlot());
> >
> > +  bool checkBuiltinTargetFeatures(const FunctionDecl *TargetDecl);
> > +
> >    llvm::CallInst *EmitRuntimeCall(llvm::Value *callee,
> >                                    const Twine &name = "");
> >    llvm::CallInst *EmitRuntimeCall(llvm::Value *callee,
> >
> > Added: cfe/trunk/test/CodeGen/target-builtin-error-2.c
> > URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/target-builtin-error-2.c?rev=250473&view=auto
> >
> ==============================================================================
> > --- cfe/trunk/test/CodeGen/target-builtin-error-2.c (added)
> > +++ cfe/trunk/test/CodeGen/target-builtin-error-2.c Thu Oct 15 18:47:11
> 2015
> > @@ -0,0 +1,13 @@
> > +// RUN: %clang_cc1 %s -triple=x86_64-linux-gnu -S -verify -o -
> > +#define __MM_MALLOC_H
> > +
> > +#include <x86intrin.h>
> > +
> > +// Since we do code generation on a function level this needs to error
> out since
> > +// the subtarget feature won't be available.
> > +__m256d wombat(__m128i a) {
> > +  if (__builtin_cpu_supports("avx"))
> > +    return __builtin_ia32_cvtdq2pd256((__v4si)a); // expected-error
> {{'__builtin_ia32_cvtdq2pd256' needs target feature avx}}
> > +  else
> > +    return (__m256d){0, 0, 0, 0};
> > +}
> >
> > Added: cfe/trunk/test/CodeGen/target-builtin-error.c
> > URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/target-builtin-error.c?rev=250473&view=auto
> >
> ==============================================================================
> > --- cfe/trunk/test/CodeGen/target-builtin-error.c (added)
> > +++ cfe/trunk/test/CodeGen/target-builtin-error.c Thu Oct 15 18:47:11
> 2015
> > @@ -0,0 +1,8 @@
> > +// RUN: %clang_cc1 %s -triple=x86_64-linux-gnu -S -verify -o -
> > +#define __MM_MALLOC_H
> > +
> > +#include <x86intrin.h>
> > +
> > +__m128d foo(__m128d a, __m128d b) {
> > +  return __builtin_ia32_addsubps(b, a); // expected-error
> {{'__builtin_ia32_addsubps' needs target feature sse3}}
> > +}
> >
> > Added: cfe/trunk/test/CodeGen/target-builtin-noerror.c
> > URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/target-builtin-noerror.c?rev=250473&view=auto
> >
> ==============================================================================
> > --- cfe/trunk/test/CodeGen/target-builtin-noerror.c (added)
> > +++ cfe/trunk/test/CodeGen/target-builtin-noerror.c Thu Oct 15 18:47:11
> 2015
> > @@ -0,0 +1,30 @@
> > +// RUN: %clang_cc1 %s -triple=x86_64-linux-gnu -S -o -
> > +#define __MM_MALLOC_H
> > +
> > +#include <x86intrin.h>
> > +
> > +// No warnings.
> > +extern __m256i a;
> > +int __attribute__((target("avx"))) bar(__m256i a) {
> > +  return _mm256_extract_epi32(a, 3);
> > +}
> > +
> > +int baz() {
> > +  return bar(a);
> > +}
> > +
> > +int __attribute__((target("avx"))) qq_avx(__m256i a) {
> > +  return _mm256_extract_epi32(a, 3);
> > +}
> > +
> > +int qq_noavx() {
> > +  return 0;
> > +}
> > +
> > +extern __m256i a;
> > +int qq() {
> > +  if (__builtin_cpu_supports("avx"))
> > +    return qq_avx(a);
> > +  else
> > +    return qq_noavx();
> > +}
> >
> >
> > _______________________________________________
> > cfe-commits mailing list
> > cfe-commits at lists.llvm.org
> > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20151017/e6c65510/attachment-0001.html>


More information about the cfe-commits mailing list