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

[Eric Christopher via cfe-commits]

Hi Akira,

I don't think there's currently a good reason we don't perform a diagnostic
there, just right now we'd have to say something bland like "unable to
inline always_inline function 'foo'" without a reason. We might not have
gotten a diagnostic in the past because of compatibility reasons/transition
reasons, but even gcc warns here. Logically the programmer said
"always_inline" - if we can't we should probably say something.

-eric

On Wed, Nov 11, 2015 at 6:17 PM Akira Hatanaka  wrote:

> Currently, we inline a function only if the call to isInlineViable returns
> true, which means there are cases where we don't inline functions marked
> always_inline. Is there a reason we haven't made changes to produce any
> diagnostic in those cases? The comment also says "should be inlined
> whenever possible", which gives the impression that this is only a best
> effort attribute.
>
>
> On Wed, Nov 11, 2015 at 6:09 PM, Eric Christopher 
> wrote:
>
>>
>>
>> On Wed, Nov 11, 2015 at 6:08 PM Akira Hatanaka 
>> wrote:
>>
>>> 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?
>>>
>>>
>> Yep. And I think an error is appropriate here. The programmer said that
>> the function must always inline and it's not always being inlined.
>>
>> I think it's a larger change because it'll require some refactoring and
>> the ability to give reasons out of the always inline pass when faced with
>> cost analysis.
>>
>> -eric
>>
>>
>>> On Wed, Nov 11, 2015 at 4:48 PM, Eric Christopher 
>>> 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@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=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=252833=252834=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, 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=252833=252834=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 =
> 

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

[Eric Christopher via cfe-commits]

On Thu, Nov 12, 2015 at 11:55 AM Robinson, Paul <
paul_robin...@playstation.sony.com> wrote:

>
>
> > -Original Message-
> > From: cfe-commits [mailto:cfe-commits-boun...@lists.llvm.org] On Behalf
> Of
> > Eric Christopher via cfe-commits
> > Sent: Wednesday, November 11, 2015 4:44 PM
> > To: cfe-commits@lists.llvm.org
> > Subject: r252834 - Provide a frontend based error for always_inline
> > functions that require
> >
> > Author: echristo
> > Date: Wed Nov 11 18:44:12 2015
> > New Revision: 252834
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=252834=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
> > =252833=252834=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, 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=252833=252834
> > ew=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(TargetDecl))
> > +// If this isn't an always_inline function we can't guarantee that
> > any
> > +// function isn't being used correctly
>
> Uh... I wouldn't mind this not using no fewer negatives...
> an "only if" kind of phrasing might help.
>

Fair :). I've reworded it in r253117, that help?

-eric


> Thanks,
> --paulr
>
> > so only check if we have the
> > +// attribute and a set of target attributes that might be different
> > from
> > +// our default.
> > +if (TargetDecl->hasAttr() &&
> > +TargetDecl->hasAttr())
> > +  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=252833=
> > 252834=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 >  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 

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

[Bruno Cardoso Lopes via cfe-commits]

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
 wrote:
> Author: echristo
> Date: Wed Nov 11 18:44:12 2015
> New Revision: 252834
>
> URL: http://llvm.org/viewvc/llvm-project?rev=252834=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=252833=252834=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, 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=252833=252834=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(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() &&
> +TargetDecl->hasAttr())
> +  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=252833=252834=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  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 

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

[Eric Christopher via cfe-commits]

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 
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
>  wrote:
> > Author: echristo
> > Date: Wed Nov 11 18:44:12 2015
> > New Revision: 252834
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=252834=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=252833=252834=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, 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=252833=252834=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(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() &&
> > +TargetDecl->hasAttr())
> > +  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=252833=252834=diff
> >
> ==
> > --- 

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

[Robinson, Paul via cfe-commits]

> -Original Message-
> From: cfe-commits [mailto:cfe-commits-boun...@lists.llvm.org] On Behalf Of
> Eric Christopher via cfe-commits
> Sent: Wednesday, November 11, 2015 4:44 PM
> To: cfe-commits@lists.llvm.org
> Subject: r252834 - Provide a frontend based error for always_inline
> functions that require
> 
> Author: echristo
> Date: Wed Nov 11 18:44:12 2015
> New Revision: 252834
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=252834=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
> =252833=252834=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, 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=252833=252834
> ew=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(TargetDecl))
> +// If this isn't an always_inline function we can't guarantee that
> any
> +// function isn't being used correctly 

Uh... I wouldn't mind this not using no fewer negatives...
an "only if" kind of phrasing might help.
Thanks,
--paulr

> so only check if we have the
> +// attribute and a set of target attributes that might be different
> from
> +// our default.
> +if (TargetDecl->hasAttr() &&
> +TargetDecl->hasAttr())
> +  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=252833=
> 252834=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  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) 

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

[David Blaikie via cfe-commits]

On Wed, Nov 11, 2015 at 4:54 PM, David Blaikie  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=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=252833=252834=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, 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=252833=252834=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(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() &&
>> +TargetDecl->hasAttr())
>> +  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=252833=252834=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>  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 FeatureMap;
>> -  CGM.getFunctionFeatureMap(FeatureMap, FD);
>> -
>> -  // If we have at least one of the features in the 

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

[Akira Hatanaka via cfe-commits]

Currently, we inline a function only if the call to isInlineViable returns
true, which means there are cases where we don't inline functions marked
always_inline. Is there a reason we haven't made changes to produce any
diagnostic in those cases? The comment also says "should be inlined
whenever possible", which gives the impression that this is only a best
effort attribute.

On Wed, Nov 11, 2015 at 6:09 PM, Eric Christopher 
wrote:

>
>
> On Wed, Nov 11, 2015 at 6:08 PM Akira Hatanaka  wrote:
>
>> 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?
>>
>>
> Yep. And I think an error is appropriate here. The programmer said that
> the function must always inline and it's not always being inlined.
>
> I think it's a larger change because it'll require some refactoring and
> the ability to give reasons out of the always inline pass when faced with
> cost analysis.
>
> -eric
>
>
>> On Wed, Nov 11, 2015 at 4:48 PM, Eric Christopher 
>> 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@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=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=252833=252834=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, 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=252833=252834=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(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() &&
 +TargetDecl->hasAttr())
 +  checkTargetFeatures(E, FD);
 +
CalleeType = getContext().getCanonicalType(CalleeType);

const auto *FnType =

 Modified: cfe/trunk/lib/CodeGen/CodeGenFunction.cpp
 URL:
 

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

[Richard Smith via cfe-commits]

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=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=252833=252834=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, 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=252833=252834=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(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() &&
> +TargetDecl->hasAttr())
> +  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=252833=252834=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  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) == "")
> -

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

[Akira Hatanaka via cfe-commits]

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 
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@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=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=252833=252834=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, 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=252833=252834=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(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() &&
>> +TargetDecl->hasAttr())
>> +  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=252833=252834=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>  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 

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

[Eric Christopher via cfe-commits]

On Wed, Nov 11, 2015 at 6:08 PM Akira Hatanaka  wrote:

> 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?
>
>
Yep. And I think an error is appropriate here. The programmer said that the
function must always inline and it's not always being inlined.

I think it's a larger change because it'll require some refactoring and the
ability to give reasons out of the always inline pass when faced with cost
analysis.

-eric


> On Wed, Nov 11, 2015 at 4:48 PM, Eric Christopher 
> 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@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=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=252833=252834=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, 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=252833=252834=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(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() &&
>>> +TargetDecl->hasAttr())
>>> +  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=252833=252834=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>>  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