<div dir="ltr">I'm reasonably certain you forgot to rebuild or ran make test in the wrong directory.<div><br></div><div>Two reasons:</div><div><br></div><div>a) This is pretty much my first patch. I mean, identical and I tried that.</div><div>b) I actually tried this one and got:</div><div><br></div><div><div>Testing Time: 217.41s</div><div>********************</div><div>Failing Tests (68):</div><div> Clang :: CodeGen/3dnow-builtins.c</div><div> Clang :: CodeGen/adc-builtins.c</div><div> Clang :: CodeGen/adx-builtins.c</div><div> Clang :: CodeGen/attr-target-x86-mmx.c</div><div> Clang :: CodeGen/avx-builtins.c</div><div> Clang :: CodeGen/avx-cmp-builtins.c</div><div> Clang :: CodeGen/avx-shuffle-builtins.c</div><div> Clang :: CodeGen/avx2-builtins.c</div><div> Clang :: CodeGen/avx512bw-builtins.c</div><div> Clang :: CodeGen/avx512cdintrin.c</div><div> Clang :: CodeGen/avx512dq-builtins.c</div><div> Clang :: CodeGen/avx512er-builtins.c</div><div> Clang :: CodeGen/avx512f-builtins.c</div><div> Clang :: CodeGen/avx512vl-builtins.c</div><div> Clang :: CodeGen/avx512vlbw-builtins.c</div><div> Clang :: CodeGen/avx512vldq-builtins.c</div><div> Clang :: CodeGen/bmi-builtins.c</div><div> Clang :: CodeGen/bmi2-builtins.c</div><div> Clang :: CodeGen/builtins-x86.c</div><div> Clang :: CodeGen/f16c-builtins.c</div><div> Clang :: CodeGen/fma-builtins.c</div><div> Clang :: CodeGen/fma4-builtins.c</div><div> Clang :: CodeGen/fsgsbase-builtins.c</div><div> Clang :: CodeGen/lzcnt-builtins.c</div><div> Clang :: CodeGen/mmx-builtins.c</div><div> Clang :: CodeGen/mmx-inline-asm.c</div><div> Clang :: CodeGen/mmx-shift-with-immediate.c</div><div> Clang :: CodeGen/ms-intrinsics.c</div><div> Clang :: CodeGen/ms-mm-align.c</div><div> Clang :: CodeGen/palignr.c</div><div> Clang :: CodeGen/pclmul-builtins.c</div><div> Clang :: CodeGen/popcnt-builtins.c</div><div> Clang :: CodeGen/prefetchw-builtins.c</div><div> Clang :: CodeGen/rdrand-builtins.c</div><div> Clang :: CodeGen/rtm-builtins.c</div><div> Clang :: CodeGen/sha-builtins.c</div><div> Clang :: CodeGen/sse-builtins-dbg.c</div><div> Clang :: CodeGen/sse-builtins.c</div><div> Clang :: CodeGen/sse.c</div><div> Clang :: CodeGen/sse3-builtins.c</div><div> Clang :: CodeGen/sse41-builtins.c</div><div> Clang :: CodeGen/sse42-builtins.c</div><div> Clang :: CodeGen/sse4a-builtins.c</div><div> Clang :: CodeGen/ssse3-builtins.c</div><div> Clang :: CodeGen/target-builtin-error-2.c</div><div> Clang :: CodeGen/target-builtin-error.c</div><div> Clang :: CodeGen/target-builtin-noerror.c</div><div> Clang :: CodeGen/tbm-builtins.c</div><div> Clang :: CodeGen/vector.c</div><div> Clang :: CodeGen/x86_32-xsave.c</div><div> Clang :: CodeGen/x86_64-xsave.c</div><div> Clang :: CodeGen/xop-builtins.c</div><div> Clang :: CodeGenCXX/mangle-ms-vector-types.cpp</div><div> Clang :: Headers/c89.c</div><div> Clang :: Headers/ms-intrin.cpp</div><div> Clang :: Headers/pmmintrin.c</div><div> Clang :: Headers/wmmintrin.c</div><div> Clang :: Headers/x86-intrinsics-headers.c</div><div> Clang :: Headers/x86intrin-2.c</div><div> Clang :: Headers/x86intrin.c</div><div> Clang :: Headers/xmmintrin.c</div><div> Clang :: Integration/carbon.c</div><div> Clang :: Integration/cocoa-pch.m</div><div> Clang :: Integration/cocoa.m</div><div> Clang :: Modules/compiler_builtins.m</div><div> Clang :: Sema/builtins-x86.c</div><div> Clang :: Sema/warn-shadow-intrinsics.c</div><div> Clang :: Sema/x86-builtin-palignr.c</div><div><br></div><div> Expected Passes : 8545</div><div> Expected Failures : 15</div><div> Unsupported Tests : 15</div><div> Unexpected Failures: 68</div></div><div><br></div><div>with lots of failures that look like:</div><div><br></div><div><div>In file included from /Users/echristo/builds/build-llvm/Debug+Asserts/bin/../lib/clang/3.8.0/include/immintrin.h:143:</div><div>/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</div><div> return (__m128i)__builtin_ia32_sha1nexte((__v4si)__X, (__v4si)__Y);</div><div> ^</div></div><div><br></div><div>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.</div><div><br></div><div>-eric</div></div><br><div class="gmail_quote"><div dir="ltr">On Sat, Oct 17, 2015 at 2:11 AM Justin Bogner <<a href="mailto:mail@justinbogner.com">mail@justinbogner.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Eric Christopher <<a href="mailto:echristo@gmail.com" target="_blank">echristo@gmail.com</a>> writes:<br>
> Can't be. We don't know we're going to make the call until we code gen. :)<br>
<br>
Okay... but then why don't any tests fail with the attached? It looks<br>
like it does the right thing in a very cursory test, as well.<br>
<br>
<br>
> On Fri, Oct 16, 2015, 7:50 PM Justin Bogner <<a href="mailto:mail@justinbogner.com" target="_blank">mail@justinbogner.com</a>> wrote:<br>
><br>
>> Eric Christopher via cfe-commits <<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a>> writes:<br>
>> > Author: echristo<br>
>> > Date: Thu Oct 15 18:47:11 2015<br>
>> > New Revision: 250473<br>
>> ><br>
>> > URL: <a href="http://llvm.org/viewvc/llvm-project?rev=250473&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=250473&view=rev</a><br>
>> > Log:<br>
>> > Add an error when calling a builtin that requires features that don't<br>
>> > match the feature set of the function that they're being called from.<br>
>> ><br>
>> > This ensures that we can effectively diagnose some[1] code that would<br>
>> > instead ICE in the backend with a failure to select message.<br>
>> ><br>
>> > Example:<br>
>> ><br>
>> > __m128d foo(__m128d a, __m128d b) {<br>
>> > return __builtin_ia32_addsubps(b, a);<br>
>> > }<br>
>> ><br>
>> > compiled for normal x86_64 via:<br>
>> ><br>
>> > clang -target x86_64-linux-gnu -c<br>
>> ><br>
>> > would fail to compile in the back end because the normal subtarget<br>
>> > features for x86_64 only include sse2 and the builtin requires sse3.<br>
>> ><br>
>> > [1] We're still not erroring on:<br>
>> ><br>
>> > __m128i bar(__m128i const *p) { return _mm_lddqu_si128(p); }<br>
>> ><br>
>> > where we should fail and error on an always_inline function being<br>
>> > inlined into a function that doesn't support the subtarget features<br>
>> > required.<br>
>> ><br>
>> > Added:<br>
>> > cfe/trunk/test/CodeGen/target-builtin-error-2.c<br>
>> > cfe/trunk/test/CodeGen/target-builtin-error.c<br>
>> > cfe/trunk/test/CodeGen/target-builtin-noerror.c<br>
>> > Modified:<br>
>> > cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td<br>
>> > cfe/trunk/lib/CodeGen/CGBuiltin.cpp<br>
>> > cfe/trunk/lib/CodeGen/CGCall.cpp<br>
>> > cfe/trunk/lib/CodeGen/CodeGenFunction.h<br>
>> ><br>
>> > Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td<br>
>> > URL:<br>
>> <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=250473&r1=250472&r2=250473&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=250473&r1=250472&r2=250473&view=diff</a><br>
>> ><br>
>> ==============================================================================<br>
>> > --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)<br>
>> > +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Thu Oct 15<br>
>> 18:47:11 2015<br>
>> > @@ -430,6 +430,7 @@ def warn_redecl_library_builtin : Warnin<br>
>> > def err_builtin_definition : Error<"definition of builtin function %0">;<br>
>> > def err_arm_invalid_specialreg : Error<"invalid special register for<br>
>> builtin">;<br>
>> > def err_invalid_cpu_supports : Error<"invalid cpu feature string for<br>
>> builtin">;<br>
>> > +def err_builtin_needs_feature : Error<"%0 needs target feature %1">;<br>
>> > def warn_builtin_unknown : Warning<"use of unknown builtin %0">,<br>
>> > InGroup<ImplicitFunctionDeclare>, DefaultError;<br>
>> > def warn_dyn_class_memaccess : Warning<<br>
>> ><br>
>> > Modified: cfe/trunk/lib/CodeGen/CGBuiltin.cpp<br>
>> > URL:<br>
>> <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGBuiltin.cpp?rev=250473&r1=250472&r2=250473&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGBuiltin.cpp?rev=250473&r1=250472&r2=250473&view=diff</a><br>
>> ><br>
>> ==============================================================================<br>
>> > --- cfe/trunk/lib/CodeGen/CGBuiltin.cpp (original)<br>
>> > +++ cfe/trunk/lib/CodeGen/CGBuiltin.cpp Thu Oct 15 18:47:11 2015<br>
>> > @@ -21,6 +21,7 @@<br>
>> > #include "clang/Basic/TargetBuiltins.h"<br>
>> > #include "clang/Basic/TargetInfo.h"<br>
>> > #include "clang/CodeGen/CGFunctionInfo.h"<br>
>> > +#include "clang/Sema/SemaDiagnostic.h"<br>
>> > #include "llvm/ADT/StringExtras.h"<br>
>> > #include "llvm/IR/CallSite.h"<br>
>> > #include "llvm/IR/DataLayout.h"<br>
>> > @@ -288,6 +289,62 @@ Value *CodeGenFunction::EmitVAStartEnd(V<br>
>> > return Builder.CreateCall(CGM.getIntrinsic(inst), ArgValue);<br>
>> > }<br>
>> ><br>
>> > +// Returns true if we have a valid set of target features.<br>
>> > +bool CodeGenFunction::checkBuiltinTargetFeatures(<br>
>> > + const FunctionDecl *TargetDecl) {<br>
>> > + // Early exit if this is an indirect call.<br>
>> > + if (!TargetDecl)<br>
>> > + return true;<br>
>> > +<br>
>> > + // Get the current enclosing function if it exists.<br>
>> > + if (const FunctionDecl *FD =<br>
>> dyn_cast_or_null<FunctionDecl>(CurFuncDecl)) {<br>
>> > + unsigned BuiltinID = TargetDecl->getBuiltinID();<br>
>> > + const char *FeatureList =<br>
>> > + CGM.getContext().BuiltinInfo.getRequiredFeatures(BuiltinID);<br>
>> > + if (FeatureList && StringRef(FeatureList) != "") {<br>
>> > + StringRef TargetCPU = Target.getTargetOpts().CPU;<br>
>> > + llvm::StringMap<bool> FeatureMap;<br>
>> > +<br>
>> > + if (const auto *TD = FD->getAttr<TargetAttr>()) {<br>
>> > + // If we have a TargetAttr build up the feature map based on<br>
>> that.<br>
>> > + TargetAttr::ParsedTargetAttr ParsedAttr = TD->parse();<br>
>> > +<br>
>> > + // Make a copy of the features as passed on the command line<br>
>> into the<br>
>> > + // beginning of the additional features from the function to<br>
>> override.<br>
>> > + ParsedAttr.first.insert(<br>
>> > + ParsedAttr.first.begin(),<br>
>> > + Target.getTargetOpts().FeaturesAsWritten.begin(),<br>
>> > + Target.getTargetOpts().FeaturesAsWritten.end());<br>
>> > +<br>
>> > + if (ParsedAttr.second != "")<br>
>> > + TargetCPU = ParsedAttr.second;<br>
>> > +<br>
>> > + // Now populate the feature map, first with the TargetCPU which<br>
>> is<br>
>> > + // either<br>
>> > + // the default or a new one from the target attribute string.<br>
>> Then we'll<br>
>> > + // use the passed in features (FeaturesAsWritten) along with<br>
>> the new<br>
>> > + // ones<br>
>> > + // from the attribute.<br>
>> > + Target.initFeatureMap(FeatureMap, CGM.getDiags(), TargetCPU,<br>
>> > + ParsedAttr.first);<br>
>> > + } else {<br>
>> > + Target.initFeatureMap(FeatureMap, CGM.getDiags(), TargetCPU,<br>
>> > + Target.getTargetOpts().Features);<br>
>> > + }<br>
>> > +<br>
>> > + // If we have at least one of the features in the feature list<br>
>> return<br>
>> > + // true, otherwise return false.<br>
>> > + SmallVector<StringRef, 1> AttrFeatures;<br>
>> > + StringRef(FeatureList).split(AttrFeatures, ",");<br>
>> > + for (const auto &Feature : AttrFeatures)<br>
>> > + if (FeatureMap[Feature])<br>
>> > + return true;<br>
>> > + return false;<br>
>> > + }<br>
>> > + }<br>
>> > + return true;<br>
>> > +}<br>
>> > +<br>
>> > RValue CodeGenFunction::EmitBuiltinExpr(const FunctionDecl *FD,<br>
>> > unsigned BuiltinID, const<br>
>> CallExpr *E,<br>
>> > ReturnValueSlot ReturnValue) {<br>
>> > @@ -1789,6 +1846,16 @@ RValue CodeGenFunction::EmitBuiltinExpr(<br>
>> > if (getContext().BuiltinInfo.isPredefinedLibFunction(BuiltinID))<br>
>> > return emitLibraryCall(*this, FD, E,<br>
>> EmitScalarExpr(E->getCallee()));<br>
>> ><br>
>> > + // Check that a call to a target specific builtin has the correct<br>
>> target<br>
>> > + // features.<br>
>> > + // This is down here to avoid non-target specific builtins, however,<br>
>> if<br>
>> > + // generic builtins start to require generic target features then we<br>
>> > + // can move this up to the beginning of the function.<br>
>> > + if (!checkBuiltinTargetFeatures(FD))<br>
>> > + CGM.getDiags().Report(E->getLocStart(),<br>
>> diag::err_builtin_needs_feature)<br>
>> > + << FD->getDeclName()<br>
>> > + << CGM.getContext().BuiltinInfo.getRequiredFeatures(BuiltinID);<br>
>><br>
>> This check really belongs in Sema, not CodeGen. I'd expect it to be part<br>
>> of CheckBuiltinFunctionCall in SemaChecking.cpp.<br>
>><br>
>> > +<br>
>> > // See if we have a target specific intrinsic.<br>
>> > const char *Name = getContext().BuiltinInfo.getName(BuiltinID);<br>
>> > Intrinsic::ID IntrinsicID = Intrinsic::not_intrinsic;<br>
>> ><br>
>> > Modified: cfe/trunk/lib/CodeGen/CGCall.cpp<br>
>> > URL:<br>
>> <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGCall.cpp?rev=250473&r1=250472&r2=250473&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGCall.cpp?rev=250473&r1=250472&r2=250473&view=diff</a><br>
>> ><br>
>> ==============================================================================<br>
>> > --- cfe/trunk/lib/CodeGen/CGCall.cpp (original)<br>
>> > +++ cfe/trunk/lib/CodeGen/CGCall.cpp Thu Oct 15 18:47:11 2015<br>
>> > @@ -21,6 +21,7 @@<br>
>> > #include "clang/AST/Decl.h"<br>
>> > #include "clang/AST/DeclCXX.h"<br>
>> > #include "clang/AST/DeclObjC.h"<br>
>> > +#include "clang/Basic/TargetBuiltins.h"<br>
>> > #include "clang/Basic/TargetInfo.h"<br>
>> > #include "clang/CodeGen/CGFunctionInfo.h"<br>
>> > #include "clang/Frontend/CodeGenOptions.h"<br>
>> ><br>
>> > Modified: cfe/trunk/lib/CodeGen/CodeGenFunction.h<br>
>> > URL:<br>
>> <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenFunction.h?rev=250473&r1=250472&r2=250473&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenFunction.h?rev=250473&r1=250472&r2=250473&view=diff</a><br>
>> ><br>
>> ==============================================================================<br>
>> > --- cfe/trunk/lib/CodeGen/CodeGenFunction.h (original)<br>
>> > +++ cfe/trunk/lib/CodeGen/CodeGenFunction.h Thu Oct 15 18:47:11 2015<br>
>> > @@ -2633,6 +2633,8 @@ public:<br>
>> > RValue EmitCallExpr(const CallExpr *E,<br>
>> > ReturnValueSlot ReturnValue = ReturnValueSlot());<br>
>> ><br>
>> > + bool checkBuiltinTargetFeatures(const FunctionDecl *TargetDecl);<br>
>> > +<br>
>> > llvm::CallInst *EmitRuntimeCall(llvm::Value *callee,<br>
>> > const Twine &name = "");<br>
>> > llvm::CallInst *EmitRuntimeCall(llvm::Value *callee,<br>
>> ><br>
>> > Added: cfe/trunk/test/CodeGen/target-builtin-error-2.c<br>
>> > URL:<br>
>> <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/target-builtin-error-2.c?rev=250473&view=auto" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/target-builtin-error-2.c?rev=250473&view=auto</a><br>
>> ><br>
>> ==============================================================================<br>
>> > --- cfe/trunk/test/CodeGen/target-builtin-error-2.c (added)<br>
>> > +++ cfe/trunk/test/CodeGen/target-builtin-error-2.c Thu Oct 15 18:47:11<br>
>> 2015<br>
>> > @@ -0,0 +1,13 @@<br>
>> > +// RUN: %clang_cc1 %s -triple=x86_64-linux-gnu -S -verify -o -<br>
>> > +#define __MM_MALLOC_H<br>
>> > +<br>
>> > +#include <x86intrin.h><br>
>> > +<br>
>> > +// Since we do code generation on a function level this needs to error<br>
>> out since<br>
>> > +// the subtarget feature won't be available.<br>
>> > +__m256d wombat(__m128i a) {<br>
>> > + if (__builtin_cpu_supports("avx"))<br>
>> > + return __builtin_ia32_cvtdq2pd256((__v4si)a); // expected-error<br>
>> {{'__builtin_ia32_cvtdq2pd256' needs target feature avx}}<br>
>> > + else<br>
>> > + return (__m256d){0, 0, 0, 0};<br>
>> > +}<br>
>> ><br>
>> > Added: cfe/trunk/test/CodeGen/target-builtin-error.c<br>
>> > URL:<br>
>> <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/target-builtin-error.c?rev=250473&view=auto" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/target-builtin-error.c?rev=250473&view=auto</a><br>
>> ><br>
>> ==============================================================================<br>
>> > --- cfe/trunk/test/CodeGen/target-builtin-error.c (added)<br>
>> > +++ cfe/trunk/test/CodeGen/target-builtin-error.c Thu Oct 15 18:47:11<br>
>> 2015<br>
>> > @@ -0,0 +1,8 @@<br>
>> > +// RUN: %clang_cc1 %s -triple=x86_64-linux-gnu -S -verify -o -<br>
>> > +#define __MM_MALLOC_H<br>
>> > +<br>
>> > +#include <x86intrin.h><br>
>> > +<br>
>> > +__m128d foo(__m128d a, __m128d b) {<br>
>> > + return __builtin_ia32_addsubps(b, a); // expected-error<br>
>> {{'__builtin_ia32_addsubps' needs target feature sse3}}<br>
>> > +}<br>
>> ><br>
>> > Added: cfe/trunk/test/CodeGen/target-builtin-noerror.c<br>
>> > URL:<br>
>> <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/target-builtin-noerror.c?rev=250473&view=auto" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/target-builtin-noerror.c?rev=250473&view=auto</a><br>
>> ><br>
>> ==============================================================================<br>
>> > --- cfe/trunk/test/CodeGen/target-builtin-noerror.c (added)<br>
>> > +++ cfe/trunk/test/CodeGen/target-builtin-noerror.c Thu Oct 15 18:47:11<br>
>> 2015<br>
>> > @@ -0,0 +1,30 @@<br>
>> > +// RUN: %clang_cc1 %s -triple=x86_64-linux-gnu -S -o -<br>
>> > +#define __MM_MALLOC_H<br>
>> > +<br>
>> > +#include <x86intrin.h><br>
>> > +<br>
>> > +// No warnings.<br>
>> > +extern __m256i a;<br>
>> > +int __attribute__((target("avx"))) bar(__m256i a) {<br>
>> > + return _mm256_extract_epi32(a, 3);<br>
>> > +}<br>
>> > +<br>
>> > +int baz() {<br>
>> > + return bar(a);<br>
>> > +}<br>
>> > +<br>
>> > +int __attribute__((target("avx"))) qq_avx(__m256i a) {<br>
>> > + return _mm256_extract_epi32(a, 3);<br>
>> > +}<br>
>> > +<br>
>> > +int qq_noavx() {<br>
>> > + return 0;<br>
>> > +}<br>
>> > +<br>
>> > +extern __m256i a;<br>
>> > +int qq() {<br>
>> > + if (__builtin_cpu_supports("avx"))<br>
>> > + return qq_avx(a);<br>
>> > + else<br>
>> > + return qq_noavx();<br>
>> > +}<br>
>> ><br>
>> ><br>
>> > _______________________________________________<br>
>> > cfe-commits mailing list<br>
>> > <a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a><br>
>> > <a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits</a><br>
>><br>
</blockquote></div>