Hi Honza,
I have isolated the ipa-inline.c part into a separate patch with a
test and attached it here. The patch is simple. Could you please take
a look?
* ipa-inline.c (can_early_inline_edge_p): Flag an error when
the function that cannot be inlined is target specific.
* gcc.target/i386/inline_error.c: New test.
Thanks
Sri
On Mon, Jun 10, 2013 at 4:10 PM, Sriraman Tallam <tmsri...@google.com> wrote:
> Ping.
>
> On Tue, Jun 4, 2013 at 2:41 PM, Sriraman Tallam <tmsri...@google.com> wrote:
>> Ping.
>>
>> On Thu, May 23, 2013 at 2:41 PM, Sriraman Tallam <tmsri...@google.com> wrote:
>>> Ping, for review of ipa-inline.c change.
>>>
>>> Sri
>>>
>>> On Mon, May 20, 2013 at 11:04 AM, Sriraman Tallam <tmsri...@google.com>
>>> wrote:
>>>> On Fri, May 17, 2013 at 11:21 PM, Jakub Jelinek <ja...@redhat.com> wrote:
>>>>> On Fri, May 17, 2013 at 09:00:21PM -0700, Sriraman Tallam wrote:
>>>>>> --- ipa-inline.c (revision 198950)
>>>>>> +++ ipa-inline.c (working copy)
>>>>>> @@ -374,7 +374,33 @@ can_early_inline_edge_p (struct cgraph_edge *e)
>>>>>> return false;
>>>>>> }
>>>>>> if (!can_inline_edge_p (e, true))
>>>>>> - return false;
>>>>>> + {
>>>>>> + enum availability avail;
>>>>>> + struct cgraph_node *callee
>>>>>> + = cgraph_function_or_thunk_node (e->callee, &avail);
>>>>>> + /* Flag an error when the inlining cannot happen because of
>>>>>> target option
>>>>>> + mismatch but the callee is marked as "always_inline". In -O0 mode
>>>>>> + this will go undetected because the error flagged in
>>>>>> + "expand_call_inline" in tree-inline.c might not execute and the
>>>>>> + inlining will not happen. Then, the linker could complain about a
>>>>>> + missing body for the callee if it turned out that the callee was
>>>>>> + also marked "gnu_inline" with extern inline keyword as bodies of
>>>>>> such
>>>>>> + functions are not generated. */
>>>>>> + if ((!optimize
>>>>>> + || flag_no_inline)
>>>>>
>>>>> This should be if ((!optimize || flag_no_inline) on one line.
>>>>>
>>>>> I'd prefer also the testcase for the ICEs, something like:
>>>>>
>>>>> /* Test case to check if AVX intrinsics and function specific target
>>>>> optimizations work together. Check by including x86intrin.h */
>>>>>
>>>>> /* { dg-do compile } */
>>>>> /* { dg-options "-O2 -mno-sse -mno-avx" } */
>>>>>
>>>>> #include <x86intrin.h>
>>>>>
>>>>> __m256 a, b, c;
>>>>> void __attribute__((target ("avx")))
>>>>> foo (void)
>>>>> {
>>>>> a = _mm256_and_ps (b, c);
>>>>> }
>>>>>
>>>>> and another testcase that does:
>>>>>
>>>>> /* { dg-do compile } */
>>>>> #pragma GCC target ("mavx") /* { dg-error "whatever" } */
>>>>>
>>>>> Otherwise it looks good to me, but I'd prefer the i?86 maintainers to
>>>>> review
>>>>> it too (and Honza for ipa-inline.c?).
>>>>
>>>> Honza, could you please take a look at the ipa-inline.c fix? I will
>>>> split the patches and submit after Honza's review. I will also make
>>>> the changes mentioned.
>>>>
>>>> Thanks
>>>> Sri
>>>>
>>>>
>>>>>
>>>>> Jakub
* ipa-inline.c (can_early_inline_edge_p): Flag an error when
the function that cannot be inlined is target specific.
* gcc.target/i386/inline_error.c: New test.
Index: testsuite/gcc.target/i386/inline_error.c
===================================================================
--- testsuite/gcc.target/i386/inline_error.c (revision 0)
+++ testsuite/gcc.target/i386/inline_error.c (revision 0)
@@ -0,0 +1,13 @@
+/* { dg-do compile } */
+/* { dg-options "-O0 -mno-popcnt" } */
+
+inline int __attribute__ ((__gnu_inline__, __always_inline__,
target("popcnt")))
+foo () /* { dg-error "inlining failed in call to extern gnu_inline .* target
specific option mismatch" } */
+{
+ return 0;
+}
+
+int bar()
+{
+ return foo ();
+} /* { dg-error "called from here" } */
Index: ipa-inline.c
===================================================================
--- ipa-inline.c (revision 200034)
+++ ipa-inline.c (working copy)
@@ -374,7 +374,31 @@ can_early_inline_edge_p (struct cgraph_edge *e)
return false;
}
if (!can_inline_edge_p (e, true))
- return false;
+ {
+ enum availability avail;
+ struct cgraph_node *callee
+ = cgraph_function_or_thunk_node (e->callee, &avail);
+ /* Flag an error here when the inlining cannot happen because of target
+ option mismatch but the callee is marked as "always_inline".
+ Otherwise, in -O0 mode this could go unreported because the error
+ flagged in "expand_call_inline" in tree-inline.c might not execute.
+ Then, the linker could complain about a missing body for the callee
+ if it turned out that the callee was also marked "gnu_inline" with
+ extern inline keyword as bodies of such functions are not
+ generated. */
+ if (!optimize
+ && e->inline_failed == CIF_TARGET_OPTION_MISMATCH
+ && lookup_attribute ("always_inline", DECL_ATTRIBUTES
(callee->symbol.decl))
+ && lookup_attribute ("gnu_inline", DECL_ATTRIBUTES
(callee->symbol.decl))
+ && DECL_DECLARED_INLINE_P (callee->symbol.decl))
+ {
+ error ("inlining failed in call to extern gnu_inline %q+F: %s",
+ callee->symbol.decl,
+ cgraph_inline_failed_string (CIF_TARGET_OPTION_MISMATCH));
+ error ("called from here");
+ }
+ return false;
+ }
return true;
}
