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

Eric Christopher via cfe-commits cfe-commits at lists.llvm.org
Fri Nov 13 20:55:43 PST 2015


This all sounds a little weird as _mm_mul_ps only requires sse. Can you
give me a testcase and command line that you're using to trigger this?

Thanks!

-eric

On Fri, Nov 13, 2015 at 8:28 PM Bruno Cardoso Lopes <bruno.cardoso at gmail.com>
wrote:

> Hi Eric,
>
> This is very nice!
>
> After the addition of this feature, LNT started crashing on x86_64h:
>
> FAIL: SingleSource/UnitTests/Vector/SSE/sse_expandfft.compile_time (3 of
> 2244)
> FAIL: SingleSource/UnitTests/Vector/SSE/sse_isamax.compile_time (4 of 2244)
> FAIL: SingleSource/UnitTests/Vector/SSE/sse_shift.compile_time (5 of 2244)
> FAIL: SingleSource/UnitTests/Vector/SSE/sse_stepfft.compile_time (6 of
> 2244)
> <... among other internal ones>
>
> The reason is that x86_64h implies "-fsgsbase", whereas SSE intrinsics
> do require it. Since this isn't the case for plain x86_64, it only
> shows up on x86_64h, example:
>
> sse.expandfft.c:195:18: error: always_inline function '_mm_mul_ps'
> requires target feature 'fsgsbase', but would be inlined into function
> 'cfft2' that is
>       compiled without support for 'fsgsbase'
>             V0 = _mm_mul_ps(V6,V3);
>
> We could annotate cfft2 with 'fsgsbase' in the tests, but it seems odd
> to me that we would need to care about any function that use x86
> intrinsics when compiling for x86_64h. Any thoughts on that?
>
>
> On Wed, Nov 11, 2015 at 4:44 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
>
>
>
> --
> Bruno Cardoso Lopes
> http://www.brunocardoso.cc
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20151114/0b6f0b7e/attachment-0001.html>


More information about the cfe-commits mailing list