RE: [PATCH 3/7]: Ping3: Merge from Stack Branch - DWARF2

["Ye, Joey" <joey dot ye at intel dot com>]

Jason wrote:
>>>> -	  gcc_assert (elim == (frame_pointer_needed ?
hard_frame_pointer_rtx
>>>> -		      : stack_pointer_rtx));
>>>> +	  gcc_assert (MAX_STACK_ALIGNMENT
>>>> +	              || elim == (frame_pointer_needed ?
hard_frame_pointer_rtx
>>>> +		                                       :
stack_pointer_rtx));
>>> I'd expect to be able to do better than this.  You're saying that if
the
>>> target supports DRAP, we can eliminate to any register?
>
>> Jason, Basically this code is saying frame pointer can be eliminated
to
>> either hard_fp or stack pointer, if target supports *stack
realignment*.
>
> That may be what you want to say, but that's not what it says...it
says
> if the target supports stack realignment, don't check what register
> we're eliminating to. 
You are right. How about this:
+	  gcc_assert ( (MAX_STACK_ALIGNMENT &&
+                               (elim == hard_frame_pointer_rtx || elim
== stack_pointer_rtx))
+	                    || elim == (frame_pointer_needed ?
hard_frame_pointer_rtx
+		                                       :
stack_pointer_rtx));

Thanks - Joey
-----Original Message-----
From: gcc-patches-owner@gcc.gnu.org
[mailto:gcc-patches-owner@gcc.gnu.org] On Behalf Of Jason Merrill
Sent: Saturday, June 07, 2008 12:36 AM
To: Guo, Xuepeng
Cc: gcc-patches@gcc.gnu.org; Richard Guenther; Lu, Hongjiu; Jan Hubicka;
Ye, Joey
Subject: Re: [PATCH 3/7]: Ping3: Merge from Stack Branch - DWARF2

Guo, Xuepeng wrote:
>> From: Jason Merrill [mailto:jason@redhat.com]
>> Also, all these rules seem to assume that the DRAP register will be
FP.

> The DRAP register is different from FP. They are used for different
purpose in our stack realign proposal. Let me explain them through a
piece of code in prologue:
>  8048620 <_Z3foov>:
>  8048620:       8d 4c 24 04             lea    0x4(%esp),%ecx
>  8048624:       83 e4 e0                and    $0xffffffe0,%esp
>  8048627:       ff 71 fc                pushl  0xfffffffc(%ecx)
>  804862a:       55                      push   %ebp
>  804862b:       89 e5                   mov    %esp,%ebp
>  804862d:       83 ec 48                sub    $0x48,%esp
> The DRAP register is ecx. Rule 19 is corresponding to asm statement
804862b. We don't assume the DRAP register is FP, we just update the
cfa.register to FP here. 

I see, and this won't be caught by the existing rules because rule 4
previously set cfa.reg to be %ecx.

But hold on a minute...we don't want the CFA to point to the realigned
stack pointer, which is what rule 19 is doing; the CFA is supposed to be
a rule for finding the top of the call frame, so in this function rule 4
is doing the right thing, and then rule 16 is messing things up.

The DWARF spec says "by definition, the CFA value does not change."

>[...] 
> To generation dwarf expression like below:
> DW_CFA_expression: r1 (DW_OP_const4s: 24; DW_OP_minus)
> I have to use this if statement to output the register number of r1
firstly.

Ah yes, you're right, DW_CFA_expression takes two operands.

>>> +  if (fde->stack_realign)
>>> +    {
>>> +    }
>>> +
>>> +  /* We need to restore register used by Dynamic Realign Argument
>>> +     Pointer through dereference.   */
>>> +  if (fde->is_drap && reg == fde->drap_regnum)
>>> +    {

> These two if statements are exclusive. One function can only fall into
one of these two if statements.

OK.

Ye, Joey wrote:
>>>> -	  gcc_assert (elim == (frame_pointer_needed ?
hard_frame_pointer_rtx
>>>> -		      : stack_pointer_rtx));
>>>> +	  gcc_assert (MAX_STACK_ALIGNMENT
>>>> +	              || elim == (frame_pointer_needed ?
hard_frame_pointer_rtx
>>>> +		                                       :
stack_pointer_rtx));
>>> I'd expect to be able to do better than this.  You're saying that if
the
>>> target supports DRAP, we can eliminate to any register?

> Jason, Basically this code is saying frame pointer can be eliminated
to
> either hard_fp or stack pointer, if target supports *stack
realignment*.

That may be what you want to say, but that's not what it says...it says
if the target supports stack realignment, don't check what register
we're eliminating to.

>>> +      /* We also need to properly restore the caller's stack
pointer.
>>> +	 But if callee calls __builtin_eh_return, the calculation of
>>> +	 offset from exception handling function (typically is
>>> +	 _Unwind_RaiseException) to target function during unwinding
>>> +	 will conflict with this mechanism.  It will simply set offset
>>> +	 to zero.  So here if functions call __builtin_eh_return,
>>> +	 there will be no unwind information for restoring the stack
>>> +	 pointer.  */
>> So this will break unwinding in the debugger out of
>> _Unwind_RaiseException?

This seems to be related to the problem with CFA definition above.

Jason