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
Sat Oct 17 04:28:45 PDT 2015
I'm reasonably certain you forgot to rebuild or ran make test in the wrong
directory.
Two reasons:
a) This is pretty much my first patch. I mean, identical and I tried that.
b) I actually tried this one and got:
Testing Time: 217.41s
********************
Failing Tests (68):
Clang :: CodeGen/3dnow-builtins.c
Clang :: CodeGen/adc-builtins.c
Clang :: CodeGen/adx-builtins.c
Clang :: CodeGen/attr-target-x86-mmx.c
Clang :: CodeGen/avx-builtins.c
Clang :: CodeGen/avx-cmp-builtins.c
Clang :: CodeGen/avx-shuffle-builtins.c
Clang :: CodeGen/avx2-builtins.c
Clang :: CodeGen/avx512bw-builtins.c
Clang :: CodeGen/avx512cdintrin.c
Clang :: CodeGen/avx512dq-builtins.c
Clang :: CodeGen/avx512er-builtins.c
Clang :: CodeGen/avx512f-builtins.c
Clang :: CodeGen/avx512vl-builtins.c
Clang :: CodeGen/avx512vlbw-builtins.c
Clang :: CodeGen/avx512vldq-builtins.c
Clang :: CodeGen/bmi-builtins.c
Clang :: CodeGen/bmi2-builtins.c
Clang :: CodeGen/builtins-x86.c
Clang :: CodeGen/f16c-builtins.c
Clang :: CodeGen/fma-builtins.c
Clang :: CodeGen/fma4-builtins.c
Clang :: CodeGen/fsgsbase-builtins.c
Clang :: CodeGen/lzcnt-builtins.c
Clang :: CodeGen/mmx-builtins.c
Clang :: CodeGen/mmx-inline-asm.c
Clang :: CodeGen/mmx-shift-with-immediate.c
Clang :: CodeGen/ms-intrinsics.c
Clang :: CodeGen/ms-mm-align.c
Clang :: CodeGen/palignr.c
Clang :: CodeGen/pclmul-builtins.c
Clang :: CodeGen/popcnt-builtins.c
Clang :: CodeGen/prefetchw-builtins.c
Clang :: CodeGen/rdrand-builtins.c
Clang :: CodeGen/rtm-builtins.c
Clang :: CodeGen/sha-builtins.c
Clang :: CodeGen/sse-builtins-dbg.c
Clang :: CodeGen/sse-builtins.c
Clang :: CodeGen/sse.c
Clang :: CodeGen/sse3-builtins.c
Clang :: CodeGen/sse41-builtins.c
Clang :: CodeGen/sse42-builtins.c
Clang :: CodeGen/sse4a-builtins.c
Clang :: CodeGen/ssse3-builtins.c
Clang :: CodeGen/target-builtin-error-2.c
Clang :: CodeGen/target-builtin-error.c
Clang :: CodeGen/target-builtin-noerror.c
Clang :: CodeGen/tbm-builtins.c
Clang :: CodeGen/vector.c
Clang :: CodeGen/x86_32-xsave.c
Clang :: CodeGen/x86_64-xsave.c
Clang :: CodeGen/xop-builtins.c
Clang :: CodeGenCXX/mangle-ms-vector-types.cpp
Clang :: Headers/c89.c
Clang :: Headers/ms-intrin.cpp
Clang :: Headers/pmmintrin.c
Clang :: Headers/wmmintrin.c
Clang :: Headers/x86-intrinsics-headers.c
Clang :: Headers/x86intrin-2.c
Clang :: Headers/x86intrin.c
Clang :: Headers/xmmintrin.c
Clang :: Integration/carbon.c
Clang :: Integration/cocoa-pch.m
Clang :: Integration/cocoa.m
Clang :: Modules/compiler_builtins.m
Clang :: Sema/builtins-x86.c
Clang :: Sema/warn-shadow-intrinsics.c
Clang :: Sema/x86-builtin-palignr.c
Expected Passes : 8545
Expected Failures : 15
Unsupported Tests : 15
Unexpected Failures: 68
with lots of failures that look like:
In file included from
/Users/echristo/builds/build-llvm/Debug+Asserts/bin/../lib/clang/3.8.0/include/immintrin.h:143:
/Users/echristo/builds/build-llvm/Debug+Asserts/bin/../lib/clang/3.8.0/include/shaintrin.h:40:19:
error: '__builtin_ia32_sha1nexte' needs target feature sha
return (__m128i)__builtin_ia32_sha1nexte((__v4si)__X, (__v4si)__Y);
^
for files that include more headers than they need (think x86intrin.h being
the only header included), but not calling any of the functions and so not
using a command line option that enables every feature for the x86 target.
-eric
On Sat, Oct 17, 2015 at 2:11 AM Justin Bogner <mail at justinbogner.com> wrote:
> Eric Christopher <echristo at gmail.com> writes:
> > Can't be. We don't know we're going to make the call until we code gen.
> :)
>
> Okay... but then why don't any tests fail with the attached? It looks
> like it does the right thing in a very cursory test, as well.
>
>
> > 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/a550b710/attachment-0001.html>
More information about the cfe-commits
mailing list