r252834 - Provide a frontend based error for always_inline functions that require

Akira Hatanaka via cfe-commits cfe-commits at lists.llvm.org
Wed Nov 11 18:08:13 PST 2015


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?

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/20151111/32599f02/attachment-0001.html>


More information about the cfe-commits mailing list