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 18:09:39 PST 2015
On Wed, Nov 11, 2015 at 6:08 PM Akira Hatanaka <ahatanak at gmail.com> wrote:
> I think you are suggesting we change the inliner to produce a diagnostic
> (error or warning?) when the callee is marked always_inline and its
> function attributes are not compatible with the caller's
> (functionsHaveCompatibleAttributes returns false). Is that correct?
>
>
Yep. And I think an error is appropriate here. The programmer said that the
function must always inline and it's not always being inlined.
I think it's a larger change because it'll require some refactoring and the
ability to give reasons out of the always inline pass when faced with cost
analysis.
-eric
> On Wed, Nov 11, 2015 at 4:48 PM, Eric Christopher <echristo at gmail.com>
> wrote:
>
>> FWIW we should also have the backend avoid inlining and perhaps produce a
>> diagnostic if something makes it there as well.
>>
>> -eric
>>
>> On Wed, Nov 11, 2015 at 4:46 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
>>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20151112/3d99a3ec/attachment-0001.html>
More information about the cfe-commits
mailing list