This all sounds a little weird as _mm_mul_ps only requires sse. Can you give me a testcase and command line that you're using to trigger this?
Thanks! -eric On Fri, Nov 13, 2015 at 8:28 PM Bruno Cardoso Lopes <bruno.card...@gmail.com> wrote: > Hi Eric, > > This is very nice! > > After the addition of this feature, LNT started crashing on x86_64h: > > FAIL: SingleSource/UnitTests/Vector/SSE/sse_expandfft.compile_time (3 of > 2244) > FAIL: SingleSource/UnitTests/Vector/SSE/sse_isamax.compile_time (4 of 2244) > FAIL: SingleSource/UnitTests/Vector/SSE/sse_shift.compile_time (5 of 2244) > FAIL: SingleSource/UnitTests/Vector/SSE/sse_stepfft.compile_time (6 of > 2244) > <... among other internal ones> > > The reason is that x86_64h implies "-fsgsbase", whereas SSE intrinsics > do require it. Since this isn't the case for plain x86_64, it only > shows up on x86_64h, example: > > sse.expandfft.c:195:18: error: always_inline function '_mm_mul_ps' > requires target feature 'fsgsbase', but would be inlined into function > 'cfft2' that is > compiled without support for 'fsgsbase' > V0 = _mm_mul_ps(V6,V3); > > We could annotate cfft2 with 'fsgsbase' in the tests, but it seems odd > to me that we would need to care about any function that use x86 > intrinsics when compiling for x86_64h. Any thoughts on that? > > > On Wed, Nov 11, 2015 at 4:44 PM, Eric Christopher via cfe-commits > <cfe-commits@lists.llvm.org> wrote: > > Author: echristo > > Date: Wed Nov 11 18:44:12 2015 > > New Revision: 252834 > > > > URL: http://llvm.org/viewvc/llvm-project?rev=252834&view=rev > > Log: > > Provide a frontend based error for always_inline functions that require > > target features that the caller function doesn't provide. This matches > > the existing backend failure to inline functions that don't have > > matching target features - and diagnoses earlier in the case of > > always_inline. > > > > Fix up a few test cases that were, in fact, invalid if you tried > > to generate code from the backend with the specified target features > > and add a couple of tests to illustrate what's going on. > > > > This should fix PR25246. > > > > Added: > > cfe/trunk/test/CodeGen/target-features-error-2.c > > cfe/trunk/test/CodeGen/target-features-error.c > > Modified: > > cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td > > cfe/trunk/lib/CodeGen/CGExpr.cpp > > cfe/trunk/lib/CodeGen/CodeGenFunction.cpp > > cfe/trunk/test/CodeGen/3dnow-builtins.c > > cfe/trunk/test/CodeGen/avx512vl-builtins.c > > > > Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td > > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=252834&r1=252833&r2=252834&view=diff > > > ============================================================================== > > --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original) > > +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Wed Nov 11 > 18:44:12 2015 > > @@ -431,6 +431,9 @@ def err_builtin_definition : Error<"defi > > 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 err_function_needs_feature > > + : Error<"function %0 and always_inline callee function %1 are > required to " > > + "have matching target features">; > > def warn_builtin_unknown : Warning<"use of unknown builtin %0">, > > InGroup<ImplicitFunctionDeclare>, DefaultError; > > def warn_dyn_class_memaccess : Warning< > > > > Modified: cfe/trunk/lib/CodeGen/CGExpr.cpp > > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGExpr.cpp?rev=252834&r1=252833&r2=252834&view=diff > > > ============================================================================== > > --- cfe/trunk/lib/CodeGen/CGExpr.cpp (original) > > +++ cfe/trunk/lib/CodeGen/CGExpr.cpp Wed Nov 11 18:44:12 2015 > > @@ -3747,6 +3747,15 @@ RValue CodeGenFunction::EmitCall(QualTyp > > assert(CalleeType->isFunctionPointerType() && > > "Call must have function pointer type!"); > > > > + if (const FunctionDecl *FD = > dyn_cast_or_null<FunctionDecl>(TargetDecl)) > > + // If this isn't an always_inline function we can't guarantee that > any > > + // function isn't being used correctly so only check if we have the > > + // attribute and a set of target attributes that might be different > from > > + // our default. > > + if (TargetDecl->hasAttr<AlwaysInlineAttr>() && > > + TargetDecl->hasAttr<TargetAttr>()) > > + checkTargetFeatures(E, FD); > > + > > CalleeType = getContext().getCanonicalType(CalleeType); > > > > const auto *FnType = > > > > Modified: cfe/trunk/lib/CodeGen/CodeGenFunction.cpp > > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenFunction.cpp?rev=252834&r1=252833&r2=252834&view=diff > > > ============================================================================== > > --- cfe/trunk/lib/CodeGen/CodeGenFunction.cpp (original) > > +++ cfe/trunk/lib/CodeGen/CodeGenFunction.cpp Wed Nov 11 18:44:12 2015 > > @@ -1843,7 +1843,8 @@ template void CGBuilderInserter<Preserve > > llvm::BasicBlock::iterator InsertPt) const; > > #undef PreserveNames > > > > -// Returns true if we have a valid set of target features. > > +// Emits an error if we don't have a valid set of target features for > the > > +// called function. > > void CodeGenFunction::checkTargetFeatures(const CallExpr *E, > > const FunctionDecl > *TargetDecl) { > > // Early exit if this is an indirect call. > > @@ -1856,31 +1857,70 @@ void CodeGenFunction::checkTargetFeature > > if (!FD) > > return; > > > > + // Grab the required features for the call. For a builtin this is > listed in > > + // the td file with the default cpu, for an always_inline function > this is any > > + // listed cpu and any listed features. > > unsigned BuiltinID = TargetDecl->getBuiltinID(); > > - const char *FeatureList = > > - CGM.getContext().BuiltinInfo.getRequiredFeatures(BuiltinID); > > - > > - if (!FeatureList || StringRef(FeatureList) == "") > > - return; > > - > > - llvm::StringMap<bool> FeatureMap; > > - CGM.getFunctionFeatureMap(FeatureMap, FD); > > - > > - // 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, ","); > > - if (!std::all_of(AttrFeatures.begin(), AttrFeatures.end(), > > - [&](StringRef &Feature) { > > - SmallVector<StringRef, 1> OrFeatures; > > - Feature.split(OrFeatures, "|"); > > - return std::any_of(OrFeatures.begin(), > OrFeatures.end(), > > - [&](StringRef &Feature) { > > - return FeatureMap[Feature]; > > - }); > > - })) > > - CGM.getDiags().Report(E->getLocStart(), > diag::err_builtin_needs_feature) > > - << TargetDecl->getDeclName() > > - << CGM.getContext().BuiltinInfo.getRequiredFeatures(BuiltinID); > > + if (BuiltinID) { > > + SmallVector<StringRef, 1> ReqFeatures; > > + const char *FeatureList = > > + CGM.getContext().BuiltinInfo.getRequiredFeatures(BuiltinID); > > + // Return if the builtin doesn't have any required features. > > + if (!FeatureList || StringRef(FeatureList) == "") > > + return; > > + StringRef(FeatureList).split(ReqFeatures, ","); > > + > > + // If there aren't any required features listed then go ahead and > return. > > + if (ReqFeatures.empty()) > > + return; > > + > > + // Now build up the set of caller features and verify that all the > required > > + // features are there. > > + llvm::StringMap<bool> CallerFeatureMap; > > + CGM.getFunctionFeatureMap(CallerFeatureMap, FD); > > + > > + // If we have at least one of the features in the feature list > return > > + // true, otherwise return false. > > + if (!std::all_of( > > + ReqFeatures.begin(), ReqFeatures.end(), [&](StringRef > &Feature) { > > + SmallVector<StringRef, 1> OrFeatures; > > + Feature.split(OrFeatures, "|"); > > + return std::any_of(OrFeatures.begin(), OrFeatures.end(), > > + [&](StringRef &Feature) { > > + return > CallerFeatureMap.lookup(Feature); > > + }); > > + })) > > + CGM.getDiags().Report(E->getLocStart(), > diag::err_builtin_needs_feature) > > + << TargetDecl->getDeclName() > > + << > CGM.getContext().BuiltinInfo.getRequiredFeatures(BuiltinID); > > + > > + } else if (TargetDecl->hasAttr<TargetAttr>()) { > > + // Get the required features for the callee. > > + SmallVector<StringRef, 1> ReqFeatures; > > + llvm::StringMap<bool> CalleeFeatureMap; > > + CGM.getFunctionFeatureMap(CalleeFeatureMap, TargetDecl); > > + for (const auto &F : CalleeFeatureMap) > > + ReqFeatures.push_back(F.getKey()); > > + // If there aren't any required features listed then go ahead and > return. > > + if (ReqFeatures.empty()) > > + return; > > + > > + // Now get the features that the caller provides. > > + llvm::StringMap<bool> CallerFeatureMap; > > + CGM.getFunctionFeatureMap(CallerFeatureMap, FD); > > + > > + // If we have at least one of the features in the feature list > return > > + // true, otherwise return false. > > + if (!std::all_of( > > + ReqFeatures.begin(), ReqFeatures.end(), [&](StringRef > &Feature) { > > + SmallVector<StringRef, 1> OrFeatures; > > + Feature.split(OrFeatures, "|"); > > + return std::any_of(OrFeatures.begin(), OrFeatures.end(), > > + [&](StringRef &Feature) { > > + return > CallerFeatureMap.lookup(Feature); > > + }); > > + })) > > + CGM.getDiags().Report(E->getLocStart(), > diag::err_function_needs_feature) > > + << FD->getDeclName() << TargetDecl->getDeclName(); > > + } > > } > > - > > > > Modified: cfe/trunk/test/CodeGen/3dnow-builtins.c > > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/3dnow-builtins.c?rev=252834&r1=252833&r2=252834&view=diff > > > ============================================================================== > > --- cfe/trunk/test/CodeGen/3dnow-builtins.c (original) > > +++ cfe/trunk/test/CodeGen/3dnow-builtins.c Wed Nov 11 18:44:12 2015 > > @@ -1,6 +1,6 @@ > > // REQUIRES: x86-registered-target > > -// RUN: %clang_cc1 %s -triple=x86_64-unknown-unknown -target-feature > +3dnow -emit-llvm -o - -Werror | FileCheck %s > > -// RUN: %clang_cc1 %s -triple=x86_64-unknown-unknown -target-feature > +3dnow -S -o - -Werror | FileCheck %s --check-prefix=CHECK-ASM > > +// RUN: %clang_cc1 %s -triple=x86_64-unknown-unknown -target-feature > +3dnowa -emit-llvm -o - -Werror | FileCheck %s > > +// RUN: %clang_cc1 %s -triple=x86_64-unknown-unknown -target-feature > +3dnowa -S -o - -Werror | FileCheck %s --check-prefix=CHECK-ASM > > > > // Don't include mm_malloc.h, it's system specific. > > #define __MM_MALLOC_H > > > > Modified: cfe/trunk/test/CodeGen/avx512vl-builtins.c > > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/avx512vl-builtins.c?rev=252834&r1=252833&r2=252834&view=diff > > > ============================================================================== > > --- cfe/trunk/test/CodeGen/avx512vl-builtins.c (original) > > +++ cfe/trunk/test/CodeGen/avx512vl-builtins.c Wed Nov 11 18:44:12 2015 > > @@ -5,102 +5,6 @@ > > > > #include <immintrin.h> > > > > -__mmask8 test_mm256_cmpeq_epi32_mask(__m256i __a, __m256i __b) { > > - // CHECK-LABEL: @test_mm256_cmpeq_epi32_mask > > - // CHECK: @llvm.x86.avx512.mask.pcmpeq.d.256 > > - return (__mmask8)_mm256_cmpeq_epi32_mask(__a, __b); > > -} > > - > > -__mmask8 test_mm256_mask_cmpeq_epi32_mask(__mmask8 __u, __m256i __a, > __m256i __b) { > > - // CHECK-LABEL: @test_mm256_mask_cmpeq_epi32_mask > > - // CHECK: @llvm.x86.avx512.mask.pcmpeq.d.256 > > - return (__mmask8)_mm256_mask_cmpeq_epi32_mask(__u, __a, __b); > > -} > > - > > -__mmask8 test_mm_cmpeq_epi32_mask(__m128i __a, __m128i __b) { > > - // CHECK-LABEL: @test_mm_cmpeq_epi32_mask > > - // CHECK: @llvm.x86.avx512.mask.pcmpeq.d.128 > > - return (__mmask8)_mm_cmpeq_epi32_mask(__a, __b); > > -} > > - > > -__mmask8 test_mm_mask_cmpeq_epi32_mask(__mmask8 __u, __m128i __a, > __m128i __b) { > > - // CHECK-LABEL: @test_mm_mask_cmpeq_epi32_mask > > - // CHECK: @llvm.x86.avx512.mask.pcmpeq.d.128 > > - return (__mmask8)_mm_mask_cmpeq_epi32_mask(__u, __a, __b); > > -} > > - > > -__mmask8 test_mm256_cmpeq_epi64_mask(__m256i __a, __m256i __b) { > > - // CHECK-LABEL: @test_mm256_cmpeq_epi64_mask > > - // CHECK: @llvm.x86.avx512.mask.pcmpeq.q.256 > > - return (__mmask8)_mm256_cmpeq_epi64_mask(__a, __b); > > -} > > - > > -__mmask8 test_mm256_mask_cmpeq_epi64_mask(__mmask8 __u, __m256i __a, > __m256i __b) { > > - // CHECK-LABEL: @test_mm256_mask_cmpeq_epi64_mask > > - // CHECK: @llvm.x86.avx512.mask.pcmpeq.q.256 > > - return (__mmask8)_mm256_mask_cmpeq_epi64_mask(__u, __a, __b); > > -} > > - > > -__mmask8 test_mm_cmpeq_epi64_mask(__m128i __a, __m128i __b) { > > - // CHECK-LABEL: @test_mm_cmpeq_epi64_mask > > - // CHECK: @llvm.x86.avx512.mask.pcmpeq.q.128 > > - return (__mmask8)_mm_cmpeq_epi64_mask(__a, __b); > > -} > > - > > -__mmask8 test_mm_mask_cmpeq_epi64_mask(__mmask8 __u, __m128i __a, > __m128i __b) { > > - // CHECK-LABEL: @test_mm_mask_cmpeq_epi64_mask > > - // CHECK: @llvm.x86.avx512.mask.pcmpeq.q.128 > > - return (__mmask8)_mm_mask_cmpeq_epi64_mask(__u, __a, __b); > > -} > > - > > -__mmask8 test_mm256_cmpgt_epi32_mask(__m256i __a, __m256i __b) { > > - // CHECK-LABEL: @test_mm256_cmpgt_epi32_mask > > - // CHECK: @llvm.x86.avx512.mask.pcmpgt.d.256 > > - return (__mmask8)_mm256_cmpgt_epi32_mask(__a, __b); > > -} > > - > > -__mmask8 test_mm256_mask_cmpgt_epi32_mask(__mmask8 __u, __m256i __a, > __m256i __b) { > > - // CHECK-LABEL: @test_mm256_mask_cmpgt_epi32_mask > > - // CHECK: @llvm.x86.avx512.mask.pcmpgt.d.256 > > - return (__mmask8)_mm256_mask_cmpgt_epi32_mask(__u, __a, __b); > > -} > > - > > -__mmask8 test_mm_cmpgt_epi32_mask(__m128i __a, __m128i __b) { > > - // CHECK-LABEL: @test_mm_cmpgt_epi32_mask > > - // CHECK: @llvm.x86.avx512.mask.pcmpgt.d.128 > > - return (__mmask8)_mm_cmpgt_epi32_mask(__a, __b); > > -} > > - > > -__mmask8 test_mm_mask_cmpgt_epi32_mask(__mmask8 __u, __m128i __a, > __m128i __b) { > > - // CHECK-LABEL: @test_mm_mask_cmpgt_epi32_mask > > - // CHECK: @llvm.x86.avx512.mask.pcmpgt.d.128 > > - return (__mmask8)_mm_mask_cmpgt_epi32_mask(__u, __a, __b); > > -} > > - > > -__mmask8 test_mm256_cmpgt_epi64_mask(__m256i __a, __m256i __b) { > > - // CHECK-LABEL: @test_mm256_cmpgt_epi64_mask > > - // CHECK: @llvm.x86.avx512.mask.pcmpgt.q.256 > > - return (__mmask8)_mm256_cmpgt_epi64_mask(__a, __b); > > -} > > - > > -__mmask8 test_mm256_mask_cmpgt_epi64_mask(__mmask8 __u, __m256i __a, > __m256i __b) { > > - // CHECK-LABEL: @test_mm256_mask_cmpgt_epi64_mask > > - // CHECK: @llvm.x86.avx512.mask.pcmpgt.q.256 > > - return (__mmask8)_mm256_mask_cmpgt_epi64_mask(__u, __a, __b); > > -} > > - > > -__mmask8 test_mm_cmpgt_epi64_mask(__m128i __a, __m128i __b) { > > - // CHECK-LABEL: @test_mm_cmpgt_epi64_mask > > - // CHECK: @llvm.x86.avx512.mask.pcmpgt.q.128 > > - return (__mmask8)_mm_cmpgt_epi64_mask(__a, __b); > > -} > > - > > -__mmask8 test_mm_mask_cmpgt_epi64_mask(__mmask8 __u, __m128i __a, > __m128i __b) { > > - // CHECK-LABEL: @test_mm_mask_cmpgt_epi64_mask > > - // CHECK: @llvm.x86.avx512.mask.pcmpgt.q.128 > > - return (__mmask8)_mm_mask_cmpgt_epi64_mask(__u, __a, __b); > > -} > > - > > __mmask8 test_mm_cmpeq_epu32_mask(__m128i __a, __m128i __b) { > > // CHECK-LABEL: @test_mm_cmpeq_epu32_mask > > // CHECK: @llvm.x86.avx512.mask.ucmp.d.128(<4 x i32> {{.*}}, <4 x > i32> {{.*}}, i32 0, i8 -1) > > > > Added: cfe/trunk/test/CodeGen/target-features-error-2.c > > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/target-features-error-2.c?rev=252834&view=auto > > > ============================================================================== > > --- cfe/trunk/test/CodeGen/target-features-error-2.c (added) > > +++ cfe/trunk/test/CodeGen/target-features-error-2.c Wed Nov 11 18:44:12 > 2015 > > @@ -0,0 +1,7 @@ > > +// RUN: %clang_cc1 %s -triple=x86_64-linux-gnu -S -verify -o - > > +#define __MM_MALLOC_H > > +#include <x86intrin.h> > > + > > +int baz(__m256i a) { > > + return _mm256_extract_epi32(a, 3); // expected-error {{function 'baz' > and always_inline callee function '_mm256_extract_epi32' are required to > have matching target features}} > > +} > > > > Added: cfe/trunk/test/CodeGen/target-features-error.c > > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/target-features-error.c?rev=252834&view=auto > > > ============================================================================== > > --- cfe/trunk/test/CodeGen/target-features-error.c (added) > > +++ cfe/trunk/test/CodeGen/target-features-error.c Wed Nov 11 18:44:12 > 2015 > > @@ -0,0 +1,8 @@ > > +// RUN: %clang_cc1 %s -triple=x86_64-linux-gnu -S -verify -o - > > +int __attribute__((target("avx"), always_inline)) foo(int a) { > > + return a + 4; > > +} > > +int bar() { > > + return foo(4); // expected-error {{function 'bar' and always_inline > callee function 'foo' are required to have matching target features}} > > +} > > + > > > > > > _______________________________________________ > > cfe-commits mailing list > > cfe-commits@lists.llvm.org > > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits > > > > -- > Bruno Cardoso Lopes > http://www.brunocardoso.cc >
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits