r252834 - Provide a frontend based error for always_inline functions that require
Eric Christopher via cfe-commits
cfe-commits at lists.llvm.org
Wed Nov 11 17:25:47 PST 2015
On Wed, Nov 11, 2015 at 5:03 PM Richard Smith <richard at metafoo.co.uk> wrote:
> 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
>
Right.
> 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?
>
So, I was waiting on this I admit :)
What we really need is a "string set" way of doing set difference and then
optionally translating that to command line options. I'm really leery of
doing the "full set of options missing" though due to just the sheer size
of the error messages out the other end. i.e. "default" compiles and
"function that requires everything a skylake processor has". An
intermediate result could be taking a look at the target attribute on the
callee and posting that as a "set" of features that are required by the
caller without bothering to do the set difference?
I'll look into doing that, though it might not be immediately.
Thoughts?
-eric
>
>
>> 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/20151112/753be187/attachment-0001.html>
More information about the cfe-commits
mailing list