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">; > 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@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