On Wed, Nov 11, 2015 at 5:13 PM David Blaikie <dblai...@gmail.com> wrote:
> On Wed, Nov 11, 2015 at 4:54 PM, David Blaikie <dblai...@gmail.com> wrote: > >> >> >> 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, ","); >>> + >>> >> >> Looks like from here on down to the end of the scope matches the tail of >> the else if clause (except for the actual diag code) - perhaps pull it out >> as a common function that returns bool & call that to then decide on the >> diagnostic? >> > > Went ahead & made this change in r252840, let me know if anything looks > off with that, etc. > > Cool deal. Thanks for the cleanup :) -eric > - Dave > > >> >> >>> + // 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 >>> >> >>
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
