r252834 - Provide a frontend based error for always_inline functions that require
Bruno Cardoso Lopes via cfe-commits
cfe-commits at lists.llvm.org
Fri Nov 13 20:27:54 PST 2015
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 at 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 at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
--
Bruno Cardoso Lopes
http://www.brunocardoso.cc
More information about the cfe-commits
mailing list