r252834 - Provide a frontend based error for always_inline functions that require
Richard Smith via cfe-commits
cfe-commits at lists.llvm.org
Wed Nov 11 17:03:36 PST 2015
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">;
>
It would be useful to say which feature is missing here. Also, "matching"
implies to me that the sets must be the same, whereas I think the rule is
that the caller must at least all the features that the callee uses.
Something like: "always_inline callee function %0 uses target feature %1
that caller %2 does not require" might be more useful?
> 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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20151111/7588fded/attachment-0001.html>
More information about the cfe-commits
mailing list