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. - 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