gcc-patches@gcc.gnu.org
[Top] [All Lists]

Re: PATCH: PR target/40838: gcc shouldn't assume that the stack is ali

Subject: Re: PATCH: PR target/40838: gcc shouldn't assume that the stack is aligned
From: Richard Guenther
Date: Sat, 17 Oct 2009 21:35:31 +0200
On Sat, Oct 17, 2009 at 9:29 PM, H.J. Lu <hjl.tools@xxxxxxxxx> wrote:
> On Sat, Oct 17, 2009 at 12:20 PM, Richard Guenther
> <richard.guenther@xxxxxxxxx> wrote:
>> On Sat, Oct 17, 2009 at 9:02 PM, H.J. Lu <hjl.tools@xxxxxxxxx> wrote:
>>> On Sat, Oct 17, 2009 at 11:51 AM, Richard Guenther
>>> <richard.guenther@xxxxxxxxx> wrote:
>>>> On Sat, Oct 17, 2009 at 8:14 PM, H.J. Lu <hjl.tools@xxxxxxxxx> wrote:
>>>>> On Fri, Oct 16, 2009 at 5:59 PM, Ian Lance Taylor <iant@xxxxxxxxxx> wrote:
>>>>>> "H.J. Lu" <hjl.tools@xxxxxxxxx> writes:
>>>>>>
>>>>>>> @@ -947,6 +949,12 @@ vect_verify_datarefs_alignment (loop_vec_info 
>>>>>>> loop_vinfo, bb_vec_info bb_vinfo)
>>>>>>>              }
>>>>>>>            return false;
>>>>>>>          }
>>>>>>> +
>>>>>>> +      mode = TYPE_MODE (STMT_VINFO_VECTYPE (stmt_info));
>>>>>>> +      align = GET_MODE_ALIGNMENT (mode);
>>>>>>> +      if (cfun->hard_stack_alignment < align)
>>>>>>> +     cfun->hard_stack_alignment = align;
>>>>>>> +
>>>>>>>        if (supportable_dr_alignment != dr_aligned
>>>>>>>            && vect_print_dump_info (REPORT_ALIGNMENT))
>>>>>>>          fprintf (vect_dump, "Vectorizing an unaligned access.");
>>>>>>
>>>>>> I do not understand why this is the right place to set your new cfun
>>>>>> field.  Your patch is about a misaligned stack.  The code you are
>>>>>> changing here does not appear to have anything to do with a misaligned
>>>>>> stack.  This code is about the correct alignment of data.  The data
>>>>>> may be on the stack but it need not be.  If the data is on the stack,
>>>>>> then the compiler should be able to determine the alignment available
>>>>>> on the stack.
>>>>>
>>>>> The issue here is to set proper incoming stack alignment. We have
>>>>> to do it before RTL expansion which uses this information to check
>>>>> if stack alignment is needed. To do that, we need to know what the
>>>>> minimum stack alignment required by hardware. We collect hard
>>>>> stack alignment when we put variables on stack and check the alignment
>>>>> generated by vectorizer.
>>>>
>>>> It's surely not the right function to do this.  Nor is it a proper
>>>> interface IMHO.
>>>> Why don't you need to check whether the access is to stack memory at all?
>>>>
>>>> I think you want to instead see if any user-alignment on automatic
>>>> variables requires stack realignment.  Thus, wherever the vectorizer
>>>> sets DECL_USER_ALIGN on automatic storage.
>>>>
>>>
>>> RTL expansion may generate automatic variables from vectorizer
>>> statements while vectorizer doesn't generate any automatic variables
>>> at all. How should I handle this case?
>>
>> The same way you would handle spills or you would handle user code
>> using intrinsics.  I don't see how the vectorizer is special.
>>
>
> For
>
> ---
> void g();
>
> int p[100];
> int q[100];
>
> void f()
> {
>        int i;
>        for (i = 0; i < 100; i++) p[i] = 0;
>        g();
>        for (i = 0; i < 100; i++) q[i] = 0;
> }
> ---
>
> Vectorizor sets DECL_USER_ALIGN on p and g. But it may not generate
> any automatic variables in function f.

Right.  What matters is if the vectorizer sets that on automatic
variables of course.

> How do I know function f
> needs stack alignment before RTL expansion?

Well, you simply decide.  Then you have to cope with the consequences
of the decision - properly set the alignment of any stack temporaries (or
spill slots) you generate.

Richard.

<Prev in Thread] Current Thread [Next in Thread>