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

Reply via email to