Testing Call frame information in .debug_frame section

Testing Call frame information in .debug_frame section

[Anitha Boyapati]

Hello,

I would like to use some help with regarding to call frame information
present in .debug_frame section in DWARF - 2.

+ Firstly, I understand that .debug_frame can be used to unwind the stack.
+ Secondly, I understand that .debug_frame is optional to gdb.
http://sourceware.org/gdb/papers/unwind.html

Now I am trying to fix a bug in AVR port of gcc to emit call-frame
information (http://gcc.gnu.org/bugzilla/show_bug.cgi?id=17994). The
problem is testing.

Initially, I thought avr-gdb can be used to backtrace/up/down with the
newly generated elf file (with a .debug_frame section in it). However,
avr-gdb does not seem to use this particular section. The version is
7.0.1. I am not sure how to test the FDEs in .debug_frame section
other than manual inspection. I am sort of newbie with DWARF-2. So, I
could be missing some pointers.

Can someone suggest a strategy? or better yet can someone shed
information on which features of  gdb uses .debug_frame section.

Thanks
Anitha

Re: Testing Call frame information in .debug_frame section

[Petr Hluzín]

Hi Anitha

On 1 February 2011 14:03, Anitha Boyapati <anitha.boyapati@gmail.com> wrote:
> Hello,
>
> I would like to use some help with regarding to call frame information
> present in .debug_frame section in DWARF - 2.
>
> + Firstly, I understand that .debug_frame can be used to unwind the stack.
> + Secondly, I understand that .debug_frame is optional to gdb.
> http://sourceware.org/gdb/papers/unwind.html
>
> Now I am trying to fix a bug in AVR port of gcc to emit call-frame
> information (http://gcc.gnu.org/bugzilla/show_bug.cgi?id=17994). The
> problem is testing.
>
> Initially, I thought avr-gdb can be used to backtrace/up/down with the
> newly generated elf file (with a .debug_frame section in it). However,
> avr-gdb does not seem to use this particular section. The version is
> 7.0.1. I am not sure how to test the FDEs in .debug_frame section
> other than manual inspection. I am sort of newbie with DWARF-2. So, I
> could be missing some pointers.
>
> Can someone suggest a strategy? or better yet can someone shed
> information on which features of  gdb uses .debug_frame section.
>
> Thanks
> Anitha
>

GDB does not use CFI on Atmel AVR, see avr-tdep.c. There is only one
call to frame_unwind_append_unwinder().

I am glad to hear that GCC can somehow emit CFI data. What is the
state of the implementation?
I tried to use CFI statements in GAS (binutils), but it cannot parse
them and provides misleading diagnostics. (I even submitted a patch
for that but never received any response.)

I tried to improve GDB's ability to scan prologues a year ago, however
I ran out of enthusiasm. (The better scanner would allow stack
reconstruction even without .debug_frame/CFI info.)

I think I know where and how to hook the new CFI-based unwinder.
However I am not familiar with .debug_frame unwinding in GDB.
I am interested in writing the unwinder. I will need an example ELF
with trivial code run-able in simulator and lots of faith/enthusiasm.
(Assuming the compiler implements this spec:
http://dwarfstd.org/doc/DWARF4.pdf, or dwarf-2.0.0.pdf)

-- 
Petr Hluzin

Re: Testing Call frame information in .debug_frame section

[Anitha Boyapati]

Hello Petr,

On 13 February 2011 08:04, Petr Hluzín <petr.hluzin@gmail.com> wrote:
>
>
> GDB does not use CFI on Atmel AVR, see avr-tdep.c. There is only one
> call to frame_unwind_append_unwinder().

Yes. I have looked into the code and it looked like the support is not
present. But my understanding is still incomplete. frame-unwind.h/c
files are what I have been looking through. Even if it is supported in
other targets like ARM perhaps, how do we use that? Which of the gdb
commands' processing involves frame unwinding following dwarf2/3/4...?


>
> I am glad to hear that GCC can somehow emit CFI data. What is the
> state of the implementation?

It has macros which is defined can fill .debug_frame with CIE and FDE
information. The implementation from target side is very minimal.

* The prologue instructions that modify frame/stack pointer have to be
marked with RTX_FRAME_RELATED_P().
* The hook INCOMING_RETURN_ADDR_RTX should know how to access return
address on stack. The return address can be stored in memory or in
some register.
*  Likewise, hook INCOMING_FRAME_SP_OFFSET should specify the stack
pointer offset from its previous frame but before prologue of the
current function.

The macros are documented in stack layout and calling conventions of
gcc internals. Emitting CFI is optional in GCC. I used DWARF2 standard
and the output seemed to comply it. (For now I am manually verifying
the dump of debug_frame section)


> I tried to use CFI statements in GAS (binutils), but it cannot parse
> them and provides misleading diagnostics. (I even submitted a patch
> for that but never received any response.)

Sounds interesting. Can you provide the link?

>
> I tried to improve GDB's ability to scan prologues a year ago, however
> I ran out of enthusiasm. (The better scanner would allow stack
> reconstruction even without .debug_frame/CFI info.)
>

How does GDB currently use information from .debug_frame section? From
the chat rooms I came to know that current GDB can limp along without
CFI information. It tries to scan the prologues and epilogues and
build frame information is what I came to know. Although I did not
analyze how.

> I think I know where and how to hook the new CFI-based unwinder.
> However I am not familiar with .debug_frame unwinding in GDB.
> I am interested in writing the unwinder. I will need an example ELF
> with trivial code run-able in simulator and lots of faith/enthusiasm.
> (Assuming the compiler implements this spec:
> http://dwarfstd.org/doc/DWARF4.pdf, or dwarf-2.0.0.pdf)
>

:-)


Anitha


> --
> Petr Hluzin

Re: Testing Call frame information in .debug_frame section

[Petr Hluzín]

Hi Anitha

On 13 February 2011 10:57, Anitha Boyapati <anitha.boyapati@gmail.com> wrote:
> Hello Petr,
>
> On 13 February 2011 08:04, Petr Hluzín <petr.hluzin@gmail.com> wrote:
>>
>>
>> GDB does not use CFI on Atmel AVR, see avr-tdep.c. There is only one
>> call to frame_unwind_append_unwinder().
>
> Yes. I have looked into the code and it looked like the support is not
> present. But my understanding is still incomplete. frame-unwind.h/c
> files are what I have been looking through. Even if it is supported in
> other targets like ARM perhaps, how do we use that? Which of the gdb
> commands' processing involves frame unwinding following dwarf2/3/4...?

I do not know how is .debug_frame/dwarf/CFI used on any arch.
If an architecture uses debugging info then "backtrace" and many other
commands use the info.

>>
>> I am glad to hear that GCC can somehow emit CFI data. What is the
>> state of the implementation?
>
> ...
> The macros are documented in stack layout and calling conventions of
> gcc internals. Emitting CFI is optional in GCC. I used DWARF2 standard
> and the output seemed to comply it. (For now I am manually verifying
> the dump of debug_frame section)

So it seems to be fished in avr-gcc?

>> I tried to use CFI statements in GAS (binutils), but it cannot parse
>> them and provides misleading diagnostics. (I even submitted a patch
>> for that but never received any response.)
>
> Sounds interesting. Can you provide the link?

Just go to binutils web and search for my name in the patch tracker.
Oh wait, they do not have one. Maybe this is the reason they forgot to
reply to my patches.

Here you have it:
http://xfree86.cygwin.ru/ml/binutils/2010-08/msg00109.html
(You might also like "add pretty-printing of AVR register names":
http://xfree86.cygwin.ru/ml/binutils/2010-08/msg00108.html)

>>
>> I tried to improve GDB's ability to scan prologues a year ago, however
>> I ran out of enthusiasm. (The better scanner would allow stack
>> reconstruction even without .debug_frame/CFI info.)
>>
>
> How does GDB currently use information from .debug_frame section? From
> the chat rooms I came to know that current GDB can limp along without
> CFI information. It tries to scan the prologues and epilogues and
> build frame information is what I came to know. Although I did not
> analyze how.

It does not use .debug_frame section (on AVR). It only looks up the
starting address of current function (the prologue scanner needs just
that).

-- 
Petr Hluzin

Re: Testing Call frame information in .debug_frame section

[Tom Tromey]

Anitha> How does GDB currently use information from .debug_frame
Anitha> section?

IIUC, GDB can use this information when it is available and when the
target requests the use of the DWARF frame sniffer.  Look for calls to
dwarf2_append_unwinders; this will tell you which targets are already
capable of using it.

Petr> Just go to binutils web and search for my name in the patch tracker.
Petr> Oh wait, they do not have one. Maybe this is the reason they forgot to
Petr> reply to my patches.

You have to ping patches, not just for binutils but also gdb and gcc.
It isn't a great system but it does have its advantages.

One or two weeks after an initial submission, if there has been no
answer, just send a ping message as a follow-up to your patch.  Then do
it every week.

Tom

Re: Testing Call frame information in .debug_frame section

[Petr Hluzín]

On 14 February 2011 17:42, Tom Tromey <tromey@redhat.com> wrote:
>...
> Petr> Just go to binutils web and search for my name in the patch tracker.
> Petr> Oh wait, they do not have one. Maybe this is the reason they forgot to
> Petr> reply to my patches.
>
> You have to ping patches, not just for binutils but also gdb and gcc.
> It isn't a great system but it does have its advantages.

One advantage is that people can write replies inline into the patch.
However this practice is fairly common so I bet patch-trackers can be
configured to include the submitted patch when mailing a notification
to a mailing list.
Savannah's tools look quite capable.

Are there more advantages? Are they pretty common? Is there an
automatized solution for them, yet?

> One or two weeks after an initial submission, if there has been no
> answer, just send a ping message as a follow-up to your patch.  Then do
> it every week.

This sounds quite mechanical, boring and common to a lot of people
(submitters). Great example of task suitable for machines. (Why do you
people choose such suffering?)

-- 
Petr Hluzin

Re: Testing Call frame information in .debug_frame section

[Tom Tromey]

>>>>> "Petr" == Petr Hluzín <petr.hluzin@gmail.com> writes:

Petr> Are there more advantages? Are they pretty common? Is there an
Petr> automatized solution for them, yet?

>> One or two weeks after an initial submission, if there has been no
>> answer, just send a ping message as a follow-up to your patch.  Then do
>> it every week.

Petr> This sounds quite mechanical, boring and common to a lot of people
Petr> (submitters). Great example of task suitable for machines. (Why do you
Petr> people choose such suffering?)

I am just describing the system as it actually exists, not really
defending it or anything.  The way I look at it is that if you want to
get a patch in, you have to bear some of the burden.

gdb tried a patch tracker for a while but it didn't prove to be very
popular.  Maybe most maintainers prefer working via email; but it is
hard to know for sure.

Recently some GCC developers started using Rietveld for patch tracking
and review:

    http://gcc.gnu.org/ml/gcc/2011-01/msg00354.html

Doug suggested using it for GDB as well, but AFAIK nobody has set it up.

Maybe if you set it up, people would use it.

Tom

Re: Testing Call frame information in .debug_frame section

[Richard Henderson]

On 02/13/2011 07:10 AM, Petr Hluzín wrote:
> http://xfree86.cygwin.ru/ml/binutils/2010-08/msg00109.html

I'll agree that a better error message would be helpful.

To answer a question within that message:

> By the way: Why AVR target does not understand CFI? What needs to be
> done in binutils? And in GDB?

  TARGET_USE_CFIPOP
  DWARF2_DEFAULT_RETURN_COLUMN
  DWARF2_CIE_DATA_ALIGNMENT
  DWARF2_LINE_MIN_INSN_LENGTH

are the macros that need to be defined,

  tc_cfi_frame_initial_instructions

may be required depending on what the state of the unwind
info incoming to a function.  Have a look at tc-i386.c,
tc_x86_frame_initial_instructions for a typical stack-based
call mechanism.

For the nearly related task of dwarf2 line numbers, you need
a call to dwarf2_emit_insn emitted immediately before each
insn is added to the frags.  Again, see tc-i386.c for ideas.


r~

Re: Testing Call frame information in .debug_frame section

[Anitha Boyapati]

On 15 February 2011 23:11, Richard Henderson <rth@redhat.com> wrote:
>
> On 02/13/2011 07:10 AM, Petr Hluzín wrote:
> > http://xfree86.cygwin.ru/ml/binutils/2010-08/msg00109.html
>
> I'll agree that a better error message would be helpful.
>
> To answer a question within that message:
>
> > By the way: Why AVR target does not understand CFI? What needs to be
> > done in binutils? And in GDB?
>
>  TARGET_USE_CFIPOP
>  DWARF2_DEFAULT_RETURN_COLUMN
>  DWARF2_CIE_DATA_ALIGNMENT
>  DWARF2_LINE_MIN_INSN_LENGTH
>
> are the macros that need to be defined,

I am a little confused here. I was under the impression that changes
to GCC files alone would suffice. I am missing something here. Are the
above mentioned changes required for assembling CFI information in
assembly files in binutils?

( I see that i386 defines  them in gas)
>
>  tc_cfi_frame_initial_instructions
>
> may be required depending on what the state of the unwind
> info incoming to a function.  Have a look at tc-i386.c,
> tc_x86_frame_initial_instructions for a typical stack-based
> call mechanism.
>
> For the nearly related task of dwarf2 line numbers, you need
> a call to dwarf2_emit_insn emitted immediately before each
> insn is added to the frags.  Again, see tc-i386.c for ideas.
>
>


Anitha

Re: Testing Call frame information in .debug_frame section

[Anitha Boyapati]

On 13 February 2011 20:40, Petr Hluzín <petr.hluzin@gmail.com> wrote:

>
> Here you have it:
> http://xfree86.cygwin.ru/ml/binutils/2010-08/msg00109.html
> (You might also like "add pretty-printing of AVR register names":
> http://xfree86.cygwin.ru/ml/binutils/2010-08/msg00108.html)
>
Sorry..I was busy with lot other things that I did not try these links.

But why I get this  -'404 Not Found' error while trying to open above
messages ?!


Anitha

Re: Testing Call frame information in .debug_frame section

[Richard Henderson]

On 02/15/2011 10:09 AM, Anitha Boyapati wrote:
> I am a little confused here. I was under the impression that changes
> to GCC files alone would suffice. I am missing something here. Are the
> above mentioned changes required for assembling CFI information in
> assembly files in binutils?

GCC *can* produce cfi information by itself without assembler help,
but can produce slightly more compact cfi information *with* help.
In addition, with assembler support it's easy to write cfi info to
go along with hand-written assembly.


r~

[avr] gas support for cfi info

[Richard Henderson]

[-- Attachment #1: Type: text/plain, Size: 1441 bytes --]

On 02/15/2011 09:41 AM, Richard Henderson wrote:
> On 02/13/2011 07:10 AM, Petr Hluzín wrote:
>> http://xfree86.cygwin.ru/ml/binutils/2010-08/msg00109.html
> 
> I'll agree that a better error message would be helpful.
> 
> To answer a question within that message:
> 
>> By the way: Why AVR target does not understand CFI? What needs to be
>> done in binutils? And in GDB?
> 
>   TARGET_USE_CFIPOP
>   DWARF2_DEFAULT_RETURN_COLUMN
>   DWARF2_CIE_DATA_ALIGNMENT
>   DWARF2_LINE_MIN_INSN_LENGTH
> 
> are the macros that need to be defined,
> 
>   tc_cfi_frame_initial_instructions
> 
> may be required depending on what the state of the unwind
> info incoming to a function.  Have a look at tc-i386.c,
> tc_x86_frame_initial_instructions for a typical stack-based
> call mechanism.
> 
> For the nearly related task of dwarf2 line numbers, you need
> a call to dwarf2_emit_insn emitted immediately before each
> insn is added to the frags.  Again, see tc-i386.c for ideas.

To follow up on myself, it appears as if avr already has dwarf2
line number support, and only needs a few things in order to
enable cfi support.

CC'd to gcc and gdb because the dwarf2 register numbers for SP
and the return address column need to be coordinated.  This is
part of the target's ABI.

I've left a ??? marker for when AVR_3_BYTE_PC would be true in
gcc; I haven't tracked down how that maps into the assembler,
or even if there is a simple mapping.


r~

[-- Attachment #2: zz --]
[-- Type: text/plain, Size: 1729 bytes --]

Index: config/tc-avr.c
===================================================================
RCS file: /cvs/src/src/gas/config/tc-avr.c,v
retrieving revision 1.74
diff -u -p -r1.74 tc-avr.c
--- config/tc-avr.c	28 Jun 2010 14:06:57 -0000	1.74
+++ config/tc-avr.c	15 Feb 2011 18:52:05 -0000
@@ -24,6 +24,8 @@
 #include "as.h"
 #include "safe-ctype.h"
 #include "subsegs.h"
+#include "dw2gencfi.h"
+
 
 struct avr_opcodes_s
 {
@@ -1488,3 +1490,12 @@ avr_cons_fix_new (fragS *frag,
       exp_mod_pm = 0;
     }
 }
+
+void
+tc_cfi_frame_initial_instructions (void)
+{
+  /* ??? How do we tell if we're in 3-byte pc mode?  */
+  /* The CFA is immediately above the return address, which is on the stack. */
+  cfi_add_CFA_def_cfa (32, 2);
+  cfi_add_CFA_offset (DWARF2_DEFAULT_RETURN_COLUMN, -2);
+}
Index: config/tc-avr.h
===================================================================
RCS file: /cvs/src/src/gas/config/tc-avr.h,v
retrieving revision 1.17
diff -u -p -r1.17 tc-avr.h
--- config/tc-avr.h	27 Oct 2009 15:39:27 -0000	1.17
+++ config/tc-avr.h	15 Feb 2011 18:52:05 -0000
@@ -153,3 +153,17 @@ extern long md_pcrel_from_section (struc
 
 /* 32 bits pseudo-addresses are used on AVR.  */
 #define DWARF2_ADDR_SIZE(bfd) 4
+
+/* Enable cfi directives.  */
+#define TARGET_USE_CFIPOP 1
+
+/* The stack grows down, and is only byte aligned.  */
+#define DWARF2_CIE_DATA_ALIGNMENT -1
+
+/* Define the column that represents the PC.  */
+/* ??? This is an abi thing; coordinate with other projects.  */
+#define DWARF2_DEFAULT_RETURN_COLUMN  36
+
+/* Define a hook to setup initial CFI state.  */
+extern void tc_cfi_frame_initial_instructions (void);
+#define tc_cfi_frame_initial_instructions tc_cfi_frame_initial_instructions

Re: Testing Call frame information in .debug_frame section

[Anitha Boyapati]

On 16 February 2011 00:18, Richard Henderson <rth@redhat.com> wrote:
> On 02/15/2011 10:09 AM, Anitha Boyapati wrote:
>> I am a little confused here. I was under the impression that changes
>> to GCC files alone would suffice. I am missing something here. Are the
>> above mentioned changes required for assembling CFI information in
>> assembly files in binutils?
>
> GCC *can* produce cfi information by itself without assembler help,
> but can produce slightly more compact cfi information *with* help.
> In addition, with assembler support it's easy to write cfi info to
> go along with hand-written assembly.

Ok.  I was skimming through binutils ml archives for more info.

Basically I have implemented CFI emission in AVR-GCC
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=17994

I have no idea that somewhat similar changes are required in binutils
too. Upon that the manual examination of output appeared correct. I am
using binutils 2.20.1. Will see what happens on supporting the above
macros. I wish there is some kind of updated  binutils internals
document like that of GCC's for some references.


Thanks Richard!


Anitha.

Re: Testing Call frame information in .debug_frame section

[Petr Hluzín]

On 15 February 2011 19:17, Anitha Boyapati <anitha.boyapati@gmail.com> wrote:
> On 13 February 2011 20:40, Petr Hluzín <petr.hluzin@gmail.com> wrote:
>
>>
>> Here you have it:
>> http://xfree86.cygwin.ru/ml/binutils/2010-08/msg00109.html
>> (You might also like "add pretty-printing of AVR register names":
>> http://xfree86.cygwin.ru/ml/binutils/2010-08/msg00108.html)
>>
> Sorry..I was busy with lot other things that I did not try these links.
>
> But why I get this  -'404 Not Found' error while trying to open above
> messages ?!

Wow. It was there when I copied the URL. Here you go:

[PATCH] add pretty-printing of AVR register names
http://sourceware.org/ml/binutils/2010-08/msg00108.html
[PATCH] print better diagnostic when no CFI support
http://sourceware.org/ml/binutils/2010-08/msg00109.html

-- 
Petr Hluzin

Re: [avr] gas support for cfi info

[Petr Hluzín]

On 15 February 2011 20:03, Richard Henderson <rth@redhat.com> wrote:
> On 02/15/2011 09:41 AM, Richard Henderson wrote:
>> On 02/13/2011 07:10 AM, Petr Hluzín wrote:
>>> http://xfree86.cygwin.ru/ml/binutils/2010-08/msg00109.html
>>
>> I'll agree that a better error message would be helpful.
>>
>> To answer a question within that message:
>>
>>> By the way: Why AVR target does not understand CFI? What needs to be
>>> done in binutils? And in GDB?
>>
>>   TARGET_USE_CFIPOP
>>   DWARF2_DEFAULT_RETURN_COLUMN
>>   DWARF2_CIE_DATA_ALIGNMENT
>>   DWARF2_LINE_MIN_INSN_LENGTH
>>
>> are the macros that need to be defined,
>>
>>   tc_cfi_frame_initial_instructions
>>
>> may be required depending on what the state of the unwind
>> info incoming to a function.  Have a look at tc-i386.c,
>> tc_x86_frame_initial_instructions for a typical stack-based
>> call mechanism.
>>
>> For the nearly related task of dwarf2 line numbers, you need
>> a call to dwarf2_emit_insn emitted immediately before each
>> insn is added to the frags.  Again, see tc-i386.c for ideas.
>
> To follow up on myself, it appears as if avr already has dwarf2
> line number support, and only needs a few things in order to
> enable cfi support.
>
> CC'd to gcc and gdb because the dwarf2 register numbers for SP
> and the return address column need to be coordinated.  This is
> part of the target's ABI.

In avr-tdep.c [1] near avr_dwarf_reg_to_regnum():
/* Unfortunately dwarf2 register for SP is 32.  */

(I can't help you with the value for #define DWARF2_DEFAULT_RETURN_COLUMN  36)
AFAIK there is no written ABI. Only the calling convention is
documented (and only the easy cases), the rest is in gdb/gcc/binutils
sources and people's heads.

> I've left a ??? marker for when AVR_3_BYTE_PC would be true in
> gcc; I haven't tracked down how that maps into the assembler,
> or even if there is a simple mapping.

In avr_gdbarch_init() in avr-tdep.c [1]:
  /* Avr-6 call instructions save 3 bytes.  */
  switch (info.bfd_arch_info->mach)
    ...
    case bfd_mach_avr6:
      call_length = 3;
      break;

[1] http://sourceware.org/cgi-bin/cvsweb.cgi/~checkout~/src/gdb/avr-tdep.c?rev=1.128&content-type=text/plain&cvsroot=src

-- 
Petr Hluzin

Re: [avr] gas support for cfi info

[Richard Henderson]

[-- Attachment #1: Type: text/plain, Size: 1528 bytes --]

On 02/15/2011 02:44 PM, Petr Hluzín wrote:
> In avr-tdep.c [1] near avr_dwarf_reg_to_regnum():
> /* Unfortunately dwarf2 register for SP is 32.  */

Excellent.  We're all on the same page for this.

> (I can't help you with the value for #define DWARF2_DEFAULT_RETURN_COLUMN  36)
> AFAIK there is no written ABI. Only the calling convention is
> documented (and only the easy cases), the rest is in gdb/gcc/binutils
> sources and people's heads.

As I recall, GCC defaults to using FIRST_PSEUDO_REGISTER for this,
so as to not overlap any hard registers.  I'll continue to so the same.

>   /* Avr-6 call instructions save 3 bytes.  */
>   switch (info.bfd_arch_info->mach)

Thanks.  That value is readily available in the assembler as well.

Anitha pointed out to me via gcc pr17994 that AVR uses post-decrement
for its pushes.  I had a brief read over the AVR insn manual, and it's
not crystal clear how multi-byte post-decrement pushes operate.

I've made an assumption that it happens as-if each byte is pushed
separately.  I.e.

  caller:           callee:
    save rN
    save rM
    trash    <- SP  hi(ret)  <- CFA
                    lo(ret)
                    trash    <- SP

This is the only way I can imagine that call insns interoperate with
byte push/pop insns.

All of which means that the return address is at a different offset
from the CFA than I originally thought.  This ought to be fixed in
the following.

Can someone please test these two patches and see if they actually
agree with the hardware?


r~

[-- Attachment #2: z-gas --]
[-- Type: text/plain, Size: 1947 bytes --]

Index: config/tc-avr.c
===================================================================
RCS file: /cvs/src/src/gas/config/tc-avr.c,v
retrieving revision 1.74
diff -u -p -r1.74 tc-avr.c
--- config/tc-avr.c	28 Jun 2010 14:06:57 -0000	1.74
+++ config/tc-avr.c	16 Feb 2011 17:39:55 -0000
@@ -24,6 +24,8 @@
 #include "as.h"
 #include "safe-ctype.h"
 #include "subsegs.h"
+#include "dw2gencfi.h"
+
 
 struct avr_opcodes_s
 {
@@ -1488,3 +1490,18 @@ avr_cons_fix_new (fragS *frag,
       exp_mod_pm = 0;
     }
 }
+
+void
+tc_cfi_frame_initial_instructions (void)
+{
+  /* AVR6 pushes 3 bytes for calls.  */
+  int return_size = (avr_mcu->mach == bfd_mach_avr6 ? 3 : 2);
+
+  /* The CFA is the caller's stack location before the call insn.  */
+  /* Note that the stack pointer is dwarf register number 32.  */
+  cfi_add_CFA_def_cfa (32, return_size);
+
+  /* Note that AVR consistently uses post-decrement, which means that things
+     do not line up the same way as for targers that use pre-decrement.  */
+  cfi_add_CFA_offset (DWARF2_DEFAULT_RETURN_COLUMN, 1-return_size);
+}
Index: config/tc-avr.h
===================================================================
RCS file: /cvs/src/src/gas/config/tc-avr.h,v
retrieving revision 1.17
diff -u -p -r1.17 tc-avr.h
--- config/tc-avr.h	27 Oct 2009 15:39:27 -0000	1.17
+++ config/tc-avr.h	16 Feb 2011 17:39:55 -0000
@@ -153,3 +153,16 @@ extern long md_pcrel_from_section (struc
 
 /* 32 bits pseudo-addresses are used on AVR.  */
 #define DWARF2_ADDR_SIZE(bfd) 4
+
+/* Enable cfi directives.  */
+#define TARGET_USE_CFIPOP 1
+
+/* The stack grows down, and is only byte aligned.  */
+#define DWARF2_CIE_DATA_ALIGNMENT -1
+
+/* Define the column that represents the PC.  */
+#define DWARF2_DEFAULT_RETURN_COLUMN  36
+
+/* Define a hook to setup initial CFI state.  */
+extern void tc_cfi_frame_initial_instructions (void);
+#define tc_cfi_frame_initial_instructions tc_cfi_frame_initial_instructions

[-- Attachment #3: z-gcc --]
[-- Type: text/plain, Size: 3012 bytes --]

diff --git a/gcc/config/avr/avr-protos.h b/gcc/config/avr/avr-protos.h
index 4569359..06c9254 100644
--- a/gcc/config/avr/avr-protos.h
+++ b/gcc/config/avr/avr-protos.h
@@ -109,6 +109,7 @@ extern RTX_CODE avr_normalize_condition (RTX_CODE condition);
 extern int compare_eq_p (rtx insn);
 extern void out_shift_with_cnt (const char *templ, rtx insn,
 				rtx operands[], int *len, int t_len);
+extern rtx avr_incoming_return_addr_rtx (void);
 #endif /* RTX_CODE */
 
 #ifdef HAVE_MACHINE_MODES
diff --git a/gcc/config/avr/avr.c b/gcc/config/avr/avr.c
index 30e4626..761eea3 100644
--- a/gcc/config/avr/avr.c
+++ b/gcc/config/avr/avr.c
@@ -534,6 +534,16 @@ get_sequence_length (rtx insns)
   return length;
 }
 
+/*  Implement INCOMING_RETURN_ADDR_RTX.  */
+
+rtx
+avr_incoming_return_addr_rtx (void)
+{
+  /* The return address is at the top of the stack.  Note that the push
+     was via post-decrement, which means the actual address is off by one.  */
+  return gen_frame_mem (HImode, plus_constant (stack_pointer_rtx, 1));
+}
+
 /*  Output function prologue.  */
 
 void
diff --git a/gcc/config/avr/avr.h b/gcc/config/avr/avr.h
index 9a71078..385b4e6 100644
--- a/gcc/config/avr/avr.h
+++ b/gcc/config/avr/avr.h
@@ -809,6 +809,9 @@ mmcu=*:-mmcu=%*}"
 
 #define OBJECT_FORMAT_ELF
 
+#define INCOMING_RETURN_ADDR_RTX   avr_incoming_return_addr_rtx ()
+#define INCOMING_FRAME_SP_OFFSET   (AVR_3_BYTE_PC ? 3 : 2)
+
 #define HARD_REGNO_RENAME_OK(OLD_REG, NEW_REG) \
   avr_hard_regno_rename_ok (OLD_REG, NEW_REG)
 
diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c
index fea8209..b5e660c 100644
--- a/gcc/dwarf2out.c
+++ b/gcc/dwarf2out.c
@@ -2240,7 +2240,7 @@ dwarf2out_frame_debug_cfa_restore (rtx reg, const char *label)
 	   cfa.base_offset = -cfa_store.offset
 
   Rule 11:
-  (set (mem ({pre_inc,pre_dec} sp:cfa_store.reg)) <reg>)
+  (set (mem ({pre_inc,pre_dec,post_dec} sp:cfa_store.reg)) <reg>)
   effects: cfa_store.offset += -/+ mode_size(mem)
 	   cfa.offset = cfa_store.offset if cfa.reg == sp
 	   cfa.reg = sp
@@ -2259,7 +2259,7 @@ dwarf2out_frame_debug_cfa_restore (rtx reg, const char *label)
 	   cfa.base_offset = -{cfa_store,cfa_temp}.offset
 
   Rule 14:
-  (set (mem (postinc <reg1>:cfa_temp <const_int>)) <reg2>)
+  (set (mem (post_inc <reg1>:cfa_temp <const_int>)) <reg2>)
   effects: cfa.reg = <reg1>
 	   cfa.base_offset = -cfa_temp.offset
 	   cfa_temp.offset -= mode_size(mem)
@@ -2592,6 +2592,7 @@ dwarf2out_frame_debug_expr (rtx expr, const char *label)
 	  /* Rule 11 */
 	case PRE_INC:
 	case PRE_DEC:
+	case POST_DEC:
 	  offset = GET_MODE_SIZE (GET_MODE (dest));
 	  if (GET_CODE (XEXP (dest, 0)) == PRE_INC)
 	    offset = -offset;
@@ -2616,7 +2617,10 @@ dwarf2out_frame_debug_expr (rtx expr, const char *label)
 	  if (cfa.reg == STACK_POINTER_REGNUM)
 	    cfa.offset = cfa_store.offset;
 
-	  offset = -cfa_store.offset;
+	  if (GET_CODE (XEXP (dest, 0)) == POST_DEC)
+	    offset += -cfa_store.offset;
+	  else
+	    offset = -cfa_store.offset;
 	  break;
 
 	  /* Rule 12 */

Re: [avr] gas support for cfi info

[Petr Hluzín]

On 16 February 2011 18:58, Richard Henderson <rth@redhat.com> wrote:
> Anitha pointed out to me via gcc pr17994 that AVR uses post-decrement
> for its pushes.  I had a brief read over the AVR insn manual, and it's
> not crystal clear how multi-byte post-decrement pushes operate.
>
> I've made an assumption that it happens as-if each byte is pushed
> separately.  I.e.
>
>  caller:           callee:
>    save rN
>    save rM
>    trash    <- SP  hi(ret)  <- CFA
>                    lo(ret)
>                    trash    <- SP
>
> This is the only way I can imagine that call insns interoperate with
> byte push/pop insns.

Yes, except the bytes of return address are in big-endian. See
avr-tdep.c in function avr_frame_prev_register():
/* ...
And to confuse matters even more, the return address stored
on the stack is in big endian byte order, even though most
everything else about the avr is little endian. Ick!  */

> All of which means that the return address is at a different offset
> from the CFA than I originally thought.  This ought to be fixed in
> the following.
>
> Can someone please test these two patches and see if they actually
> agree with the hardware?

What should I look for when testing?
The layout of stack you draw is correct (minus the endianity), see avr-tdep.c:
/* Any function with a frame looks like this ...

-- 
Petr Hluzin

Re: [avr] gas support for cfi info

[Anitha Boyapati]

[-- Attachment #1: Type: text/plain, Size: 2931 bytes --]

On 16 February 2011 23:28, Richard Henderson <rth@redhat.com> wrote:
> On 02/15/2011 02:44 PM, Petr Hluzín wrote:
>> In avr-tdep.c [1] near avr_dwarf_reg_to_regnum():
>> /* Unfortunately dwarf2 register for SP is 32.  */
>
> Excellent.  We're all on the same page for this.
>
>> (I can't help you with the value for #define DWARF2_DEFAULT_RETURN_COLUMN  36)
>> AFAIK there is no written ABI. Only the calling convention is
>> documented (and only the easy cases), the rest is in gdb/gcc/binutils
>> sources and people's heads.
>
> As I recall, GCC defaults to using FIRST_PSEUDO_REGISTER for this,
> so as to not overlap any hard registers.  I'll continue to so the same.
>
>>   /* Avr-6 call instructions save 3 bytes.  */
>>   switch (info.bfd_arch_info->mach)
>
> Thanks.  That value is readily available in the assembler as well.
>
> Anitha pointed out to me via gcc pr17994 that AVR uses post-decrement
> for its pushes.  I had a brief read over the AVR insn manual, and it's
> not crystal clear how multi-byte post-decrement pushes operate.
>
> I've made an assumption that it happens as-if each byte is pushed
> separately.  I.e.
>
>  caller:           callee:
>    save rN
>    save rM
>    trash    <- SP  hi(ret)  <- CFA
>                    lo(ret)
>                    trash    <- SP
>
> This is the only way I can imagine that call insns interoperate with
> byte push/pop insns.
>

The stack layout is correct. For call/rcall instructions, PC-low is
pushed first followed by a PC-high. (I just verified by
running/debugging a small app on the device)

> All of which means that the return address is at a different offset
> from the CFA than I originally thought.  This ought to be fixed in
> the following.


Can you please explain the logic behind the following lines in gcc patch:


-         offset = -cfa_store.offset;
+         if (GET_CODE (XEXP (dest, 0)) == POST_DEC)
+           offset += -cfa_store.offset;
+         else
+           offset = -cfa_store.offset;



>
> Can someone please test these two patches and see if they actually
> agree with the hardware?

I have tried only compiler patch. Please refer to the attached output
for a small testcase. (avr-objdump -WfF). It appeared correct to me.

However I have one simple question with regarding the output: The CFI
instructions for registers have changed only after the prologue. (For
convenience I have attached disassembly too). As far as I understand,
DWARF2 spec emits CFI instructions immediately. (Appendix 5 of DWARF2
specification)

The other scenario is - how about functions with signals/interrupts?
The compiler will give an ICE compiling a function as below:

void my_interrupt_handler() __attribute__ (("interrupt"));

Likewise, for signal attribute too. I am going to apply assembler
patch and test it. Will get back on it shortly.

Anitha

>
> r~
>

[-- Attachment #2: call-saved.txt --]
[-- Type: text/plain, Size: 5684 bytes --]


call-saved:     file format elf32-avr

Contents of the .debug_frame section:

00000000 00000010 ffffffff CIE
  Version:               1
  Augmentation:          ""
  Code alignment factor: 1
  Data alignment factor: -1
  Return address column: 36

  DW_CFA_def_cfa: r32 ofs 2
  DW_CFA_offset: r36 at cfa-1
  DW_CFA_nop
  DW_CFA_nop

00000014 0000006c 00000000 FDE cie=00000000 pc=00000000..000000ce
  DW_CFA_advance_loc: 2 to 00000002
  DW_CFA_def_cfa_offset: 3
  DW_CFA_advance_loc: 2 to 00000004
  DW_CFA_def_cfa_offset: 4
  DW_CFA_advance_loc: 2 to 00000006
  DW_CFA_def_cfa_offset: 5
  DW_CFA_advance_loc: 2 to 00000008
  DW_CFA_def_cfa_offset: 6
  DW_CFA_advance_loc: 2 to 0000000a
  DW_CFA_def_cfa_offset: 7
  DW_CFA_advance_loc: 2 to 0000000c
  DW_CFA_def_cfa_offset: 8
  DW_CFA_advance_loc: 2 to 0000000e
  DW_CFA_def_cfa_offset: 9
  DW_CFA_advance_loc: 2 to 00000010
  DW_CFA_def_cfa_offset: 10
  DW_CFA_advance_loc: 2 to 00000012
  DW_CFA_def_cfa_offset: 11
  DW_CFA_advance_loc: 2 to 00000014
  DW_CFA_def_cfa_offset: 12
  DW_CFA_advance_loc: 2 to 00000016
  DW_CFA_def_cfa_offset: 13
  DW_CFA_advance_loc: 2 to 00000018
  DW_CFA_def_cfa_offset: 14
  DW_CFA_advance_loc: 2 to 0000001a
  DW_CFA_def_cfa_offset: 15
  DW_CFA_advance_loc: 2 to 0000001c
  DW_CFA_def_cfa_offset: 16
  DW_CFA_advance_loc: 2 to 0000001e
  DW_CFA_def_cfa_offset: 17
  DW_CFA_advance_loc: 2 to 00000020
  DW_CFA_def_cfa_offset: 18
  DW_CFA_advance_loc: 4 to 00000024
  DW_CFA_def_cfa_offset: 20
  DW_CFA_offset: r28 at cfa-18
  DW_CFA_offset: r17 at cfa-17
  DW_CFA_offset: r16 at cfa-16
  DW_CFA_offset: r15 at cfa-15
  DW_CFA_offset: r14 at cfa-14
  DW_CFA_offset: r13 at cfa-13
  DW_CFA_offset: r12 at cfa-12
  DW_CFA_offset: r11 at cfa-11
  DW_CFA_offset: r10 at cfa-10
  DW_CFA_offset: r9 at cfa-9
  DW_CFA_offset: r8 at cfa-8
  DW_CFA_offset: r7 at cfa-7
  DW_CFA_offset: r6 at cfa-6
  DW_CFA_offset: r5 at cfa-5
  DW_CFA_offset: r4 at cfa-4
  DW_CFA_offset: r3 at cfa-3
  DW_CFA_offset: r2 at cfa-2
  DW_CFA_advance_loc: 4 to 00000028
  DW_CFA_def_cfa_register: r28
  DW_CFA_advance_loc: 2 to 0000002a
  DW_CFA_def_cfa_offset: 36
  DW_CFA_advance_loc: 10 to 00000034
  DW_CFA_def_cfa_register: r32
  DW_CFA_nop
  DW_CFA_nop

00000084 00000014 00000000 FDE cie=00000000 pc=000000ce..000000de
  DW_CFA_advance_loc: 4 to 000000d2
  DW_CFA_def_cfa_offset: 4
  DW_CFA_offset: r28 at cfa-2
  DW_CFA_advance_loc: 4 to 000000d6
  DW_CFA_def_cfa_register: r28


call-saved:     file format elf32-avr

Contents of the .debug_frame section:

00000000 00000010 ffffffff CIE "" cf=1 df=-1 ra=36
   LOC   CFA      ra      
00000000 r32+2    c-1   

00000014 0000006c 00000000 FDE cie=00000000 pc=00000000..000000ce
   LOC   CFA      r2    r3    r4    r5    r6    r7    r8    r9    r10   r11   r12   r13   r14   r15   r16   r17   r28   ra      
00000000 r32+2    u     u     u     u     u     u     u     u     u     u     u     u     u     u     u     u     u     c-1   
00000002 r32+3    u     u     u     u     u     u     u     u     u     u     u     u     u     u     u     u     u     c-1   
00000004 r32+4    u     u     u     u     u     u     u     u     u     u     u     u     u     u     u     u     u     c-1   
00000006 r32+5    u     u     u     u     u     u     u     u     u     u     u     u     u     u     u     u     u     c-1   
00000008 r32+6    u     u     u     u     u     u     u     u     u     u     u     u     u     u     u     u     u     c-1   
0000000a r32+7    u     u     u     u     u     u     u     u     u     u     u     u     u     u     u     u     u     c-1   
0000000c r32+8    u     u     u     u     u     u     u     u     u     u     u     u     u     u     u     u     u     c-1   
0000000e r32+9    u     u     u     u     u     u     u     u     u     u     u     u     u     u     u     u     u     c-1   
00000010 r32+10   u     u     u     u     u     u     u     u     u     u     u     u     u     u     u     u     u     c-1   
00000012 r32+11   u     u     u     u     u     u     u     u     u     u     u     u     u     u     u     u     u     c-1   
00000014 r32+12   u     u     u     u     u     u     u     u     u     u     u     u     u     u     u     u     u     c-1   
00000016 r32+13   u     u     u     u     u     u     u     u     u     u     u     u     u     u     u     u     u     c-1   
00000018 r32+14   u     u     u     u     u     u     u     u     u     u     u     u     u     u     u     u     u     c-1   
0000001a r32+15   u     u     u     u     u     u     u     u     u     u     u     u     u     u     u     u     u     c-1   
0000001c r32+16   u     u     u     u     u     u     u     u     u     u     u     u     u     u     u     u     u     c-1   
0000001e r32+17   u     u     u     u     u     u     u     u     u     u     u     u     u     u     u     u     u     c-1   
00000020 r32+18   u     u     u     u     u     u     u     u     u     u     u     u     u     u     u     u     u     c-1   
00000024 r32+20   c-2   c-3   c-4   c-5   c-6   c-7   c-8   c-9   c-10  c-11  c-12  c-13  c-14  c-15  c-16  c-17  c-18  c-1   
00000028 r28+20   c-2   c-3   c-4   c-5   c-6   c-7   c-8   c-9   c-10  c-11  c-12  c-13  c-14  c-15  c-16  c-17  c-18  c-1   
0000002a r28+36   c-2   c-3   c-4   c-5   c-6   c-7   c-8   c-9   c-10  c-11  c-12  c-13  c-14  c-15  c-16  c-17  c-18  c-1   
00000034 r32+36   c-2   c-3   c-4   c-5   c-6   c-7   c-8   c-9   c-10  c-11  c-12  c-13  c-14  c-15  c-16  c-17  c-18  c-1   

00000084 00000014 00000000 FDE cie=00000000 pc=000000ce..000000de
   LOC   CFA      r28   ra      
000000ce r32+2    u     c-1   
000000d2 r32+4    c-2   c-1   
000000d6 r28+4    c-2   c-1   


[-- Attachment #3: call-saved.c --]
[-- Type: text/x-csrc, Size: 281 bytes --]

int foo() {

	register a1, b1, c1, d1, e1, f1, g1, h1;
	register a2, b2, c2, d2, e2, f2, g2, h2;

	register result1 = a1+b1+c1+d1+e1+f1+g1+h1;
	register result2 = a2+b2+c2+d2+e2+f2+g2+h2;

	
	register result = result1 + result2;

	return result;
}

void main() {
	return foo();

}

[-- Attachment #4: call-saved-disas.txt --]
[-- Type: text/plain, Size: 3628 bytes --]


call-saved:     file format elf32-avr


Disassembly of section .text:

00000000 <foo>:
   0:	2f 92       	push	r2
   2:	3f 92       	push	r3
   4:	4f 92       	push	r4
   6:	5f 92       	push	r5
   8:	6f 92       	push	r6
   a:	7f 92       	push	r7
   c:	8f 92       	push	r8
   e:	9f 92       	push	r9
  10:	af 92       	push	r10
  12:	bf 92       	push	r11
  14:	cf 92       	push	r12
  16:	df 92       	push	r13
  18:	ef 92       	push	r14
  1a:	ff 92       	push	r15
  1c:	0f 93       	push	r16
  1e:	1f 93       	push	r17
  20:	df 93       	push	r29
  22:	cf 93       	push	r28
  24:	cd b7       	in	r28, 0x3d	; 61
  26:	de b7       	in	r29, 0x3e	; 62
  28:	60 97       	sbiw	r28, 0x10	; 16
  2a:	0f b6       	in	r0, 0x3f	; 63
  2c:	f8 94       	cli
  2e:	de bf       	out	0x3e, r29	; 62
  30:	0f be       	out	0x3f, r0	; 63
  32:	cd bf       	out	0x3d, r28	; 61
  34:	8e 2d       	mov	r24, r14
  36:	9f 2d       	mov	r25, r15
  38:	80 0f       	add	r24, r16
  3a:	91 1f       	adc	r25, r17
  3c:	29 81       	ldd	r18, Y+1	; 0x01
  3e:	3a 81       	ldd	r19, Y+2	; 0x02
  40:	82 0f       	add	r24, r18
  42:	93 1f       	adc	r25, r19
  44:	2b 81       	ldd	r18, Y+3	; 0x03
  46:	3c 81       	ldd	r19, Y+4	; 0x04
  48:	82 0f       	add	r24, r18
  4a:	93 1f       	adc	r25, r19
  4c:	2d 81       	ldd	r18, Y+5	; 0x05
  4e:	3e 81       	ldd	r19, Y+6	; 0x06
  50:	82 0f       	add	r24, r18
  52:	93 1f       	adc	r25, r19
  54:	2f 81       	ldd	r18, Y+7	; 0x07
  56:	38 85       	ldd	r19, Y+8	; 0x08
  58:	82 0f       	add	r24, r18
  5a:	93 1f       	adc	r25, r19
  5c:	29 85       	ldd	r18, Y+9	; 0x09
  5e:	3a 85       	ldd	r19, Y+10	; 0x0a
  60:	82 0f       	add	r24, r18
  62:	93 1f       	adc	r25, r19
  64:	eb 84       	ldd	r14, Y+11	; 0x0b
  66:	fc 84       	ldd	r15, Y+12	; 0x0c
  68:	e8 0e       	add	r14, r24
  6a:	f9 1e       	adc	r15, r25
  6c:	8d 85       	ldd	r24, Y+13	; 0x0d
  6e:	9e 85       	ldd	r25, Y+14	; 0x0e
  70:	2f 85       	ldd	r18, Y+15	; 0x0f
  72:	38 89       	ldd	r19, Y+16	; 0x10
  74:	82 0f       	add	r24, r18
  76:	93 1f       	adc	r25, r19
  78:	82 0d       	add	r24, r2
  7a:	93 1d       	adc	r25, r3
  7c:	84 0d       	add	r24, r4
  7e:	95 1d       	adc	r25, r5
  80:	86 0d       	add	r24, r6
  82:	97 1d       	adc	r25, r7
  84:	88 0d       	add	r24, r8
  86:	99 1d       	adc	r25, r9
  88:	8a 0d       	add	r24, r10
  8a:	9b 1d       	adc	r25, r11
  8c:	08 2f       	mov	r16, r24
  8e:	19 2f       	mov	r17, r25
  90:	0c 0d       	add	r16, r12
  92:	1d 1d       	adc	r17, r13
  94:	0e 0d       	add	r16, r14
  96:	1f 1d       	adc	r17, r15
  98:	80 2f       	mov	r24, r16
  9a:	91 2f       	mov	r25, r17
  9c:	60 96       	adiw	r28, 0x10	; 16
  9e:	0f b6       	in	r0, 0x3f	; 63
  a0:	f8 94       	cli
  a2:	de bf       	out	0x3e, r29	; 62
  a4:	0f be       	out	0x3f, r0	; 63
  a6:	cd bf       	out	0x3d, r28	; 61
  a8:	cf 91       	pop	r28
  aa:	df 91       	pop	r29
  ac:	1f 91       	pop	r17
  ae:	0f 91       	pop	r16
  b0:	ff 90       	pop	r15
  b2:	ef 90       	pop	r14
  b4:	df 90       	pop	r13
  b6:	cf 90       	pop	r12
  b8:	bf 90       	pop	r11
  ba:	af 90       	pop	r10
  bc:	9f 90       	pop	r9
  be:	8f 90       	pop	r8
  c0:	7f 90       	pop	r7
  c2:	6f 90       	pop	r6
  c4:	5f 90       	pop	r5
  c6:	4f 90       	pop	r4
  c8:	3f 90       	pop	r3
  ca:	2f 90       	pop	r2
  cc:	08 95       	ret

000000ce <main>:
  ce:	df 93       	push	r29
  d0:	cf 93       	push	r28
  d2:	cd b7       	in	r28, 0x3d	; 61
  d4:	de b7       	in	r29, 0x3e	; 62
  d6:	94 df       	rcall	.-216    	; 0x0 <foo>
  d8:	cf 91       	pop	r28
  da:	df 91       	pop	r29
  dc:	08 95       	ret

Re: [avr] gas support for cfi info

[Richard Henderson]

On 02/17/2011 07:35 AM, Anitha Boyapati wrote:
> Can you please explain the logic behind the following lines in gcc patch:
> 
> 
> -         offset = -cfa_store.offset;
> +         if (GET_CODE (XEXP (dest, 0)) == POST_DEC)
> +           offset += -cfa_store.offset;
> +         else
> +           offset = -cfa_store.offset;

This is differentiating between pre-dec and post-dec.

		pre-dec		post-dec
before		stuff		stuff
		stuff	<-sp	trash	<-sp
		trash

after		stuff		stuff
		stuff		value
		value	<-sp	trash	<-sp

We've just decremented cfa_store.offset by the size of the pushed
value, and we're computing the offset of VALUE from the CFA.  For
pre-decrement, the value is stored at the CFA offset (the else);
for post-decrement, the value is stored just above the CFA offset.

I admit the logic is confusing here, because we're storing some
quantities as positive offsets, and some quantities as negative
offsets, and we're also using the same variable for two different
things over two sections of code.  Perhaps it would have been less
obtuse if I had written

	offset = -(cfa_store.offset - offset);

> However I have one simple question with regarding the output: The CFI
> instructions for registers have changed only after the prologue. (For
> convenience I have attached disassembly too). As far as I understand,
> DWARF2 spec emits CFI instructions immediately. (Appendix 5 of DWARF2
> specification)

GCC is attempting to minimize the number of advance opcodes by grouping
the DW_CFA_offset opcodes.  We can delay emission of such until the 
registers in question are actually clobbered.  In this case this delay
tactic failed because of the push -- we cannot delay changes to the CFA.

> The other scenario is - how about functions with signals/interrupts?
> The compiler will give an ICE compiling a function as below:
> 
> void my_interrupt_handler() __attribute__ (("interrupt"));

It's a bug in the avr backend; the enable_interrupt insn should not be
marked as frame-related.  We should fix this separately.


r~

Re: [avr] gas support for cfi info

[Richard Henderson]

On 02/16/2011 02:47 PM, Petr Hluzín wrote:
> What should I look for when testing?

Run the gdb testsuite with dwarf-3 enabled.  Either by editing the
default in the compiler, or by some dejagnu argument that compiles
the tests with -gdwarf-3.

The use of Dwarf3 enables the use of DW_OP_call_frame_cfa in the
DW_AT_frame_base field, which means that the .debug_frame info will
be used every time local variables are referenced, as well as for
unwinding the stack.

If lots of tests fail, see if you can determine if the address
computed for the local variable -- or even more particularly a
function parameter -- is off by a byte.  I have an idea that we're
missing a definition of

#define ARG_POINTER_CFA_OFFSET(FNDECL) -1

in GCC.



r~

Re: [avr] gas support for cfi info

[Tristan Gingold]

On Feb 17, 2011, at 5:12 PM, Richard Henderson wrote:

> On 02/16/2011 02:47 PM, Petr Hluzín wrote:
>> What should I look for when testing?
> 
> Run the gdb testsuite with dwarf-3 enabled.  Either by editing the
> default in the compiler, or by some dejagnu argument that compiles
> the tests with -gdwarf-3.

Note that gdb includes an avr simulator (if this could simplify your testing).

Re: [avr] gas support for cfi info

[Richard Henderson]

[-- Attachment #1: Type: text/plain, Size: 1809 bytes --]

On 02/17/2011 08:04 AM, Richard Henderson wrote:
>> The other scenario is - how about functions with signals/interrupts?
>> The compiler will give an ICE compiling a function as below:
>>
>> void my_interrupt_handler() __attribute__ (("interrupt"));
> 
> It's a bug in the avr backend; the enable_interrupt insn should not be
> marked as frame-related.  We should fix this separately.

Looking at this closer, it seem that the FRAME_RELATED markers in avr.c
were sprinkled at random without really knowing what is going on.  There
were 2 places where UNSPEC_VOLATILE insns were marked frame related,
somehow expecting the unwinder to do something magical.

The following cleans all that up.  There are a couple of odd points:

  (1) SREG and RAMPZ cannot be annotated as saved in the frame of an
      interrupt function without allocating hard register numbers for
      these registers.  Not to be confused with "real" registers, 
      these need a number in the same way that SP_L and SP_H have
      register numbers.

      I've simply ignored unwind info for these for now.

  (2) At present it's possible to use epilogue_restores without
      having used prologue_saves.  I.e. use epilogue_restores with
      an inline prologue.  The problem being that the inline prologue
      uses an HImode push of REG_Y (i.e. r29 first), whereas the code
      in prologue_saves pushes r28 first and epilogue_restores is
      written to expect that.

      I've changed the inline prologue and epilogue to emit explicit
      byte push/pop for r28/r29 in numerical order to match the 
      external routines.

      This has a side effect that r29 actually receives unwind info.
      The generic unwinder doesn't expect to see multi-register
      saves, and only emits one unwind opcode for each save.


r~

[-- Attachment #2: z-gcc-2 --]
[-- Type: text/plain, Size: 20167 bytes --]

diff --git a/gcc/config/avr/avr-protos.h b/gcc/config/avr/avr-protos.h
index 4569359..06c9254 100644
--- a/gcc/config/avr/avr-protos.h
+++ b/gcc/config/avr/avr-protos.h
@@ -109,6 +109,7 @@ extern RTX_CODE avr_normalize_condition (RTX_CODE condition);
 extern int compare_eq_p (rtx insn);
 extern void out_shift_with_cnt (const char *templ, rtx insn,
 				rtx operands[], int *len, int t_len);
+extern rtx avr_incoming_return_addr_rtx (void);
 #endif /* RTX_CODE */
 
 #ifdef HAVE_MACHINE_MODES
diff --git a/gcc/config/avr/avr.c b/gcc/config/avr/avr.c
index 30e4626..6eaa593 100644
--- a/gcc/config/avr/avr.c
+++ b/gcc/config/avr/avr.c
@@ -534,6 +534,35 @@ get_sequence_length (rtx insns)
   return length;
 }
 
+/*  Implement INCOMING_RETURN_ADDR_RTX.  */
+
+rtx
+avr_incoming_return_addr_rtx (void)
+{
+  /* The return address is at the top of the stack.  Note that the push
+     was via post-decrement, which means the actual address is off by one.  */
+  return gen_frame_mem (HImode, plus_constant (stack_pointer_rtx, 1));
+}
+
+/*  Helper for expand_prologue.  Emit a push of a byte register.  */
+
+static void
+emit_push_byte (unsigned regno, bool frame_related_p)
+{
+  rtx mem, reg, insn;
+
+  mem = gen_rtx_POST_DEC (HImode, stack_pointer_rtx);
+  mem = gen_frame_mem (QImode, mem);
+  reg = gen_rtx_REG (QImode, regno);
+
+  insn = emit_insn (gen_rtx_SET (VOIDmode, mem, reg));
+  if (frame_related_p)
+    RTX_FRAME_RELATED_P (insn) = 1;
+
+  cfun->machine->stack_usage++;
+}
+
+
 /*  Output function prologue.  */
 
 void
@@ -543,11 +572,6 @@ expand_prologue (void)
   HARD_REG_SET set;
   int minimize;
   HOST_WIDE_INT size = get_frame_size();
-  /* Define templates for push instructions.  */
-  rtx pushbyte = gen_rtx_MEM (QImode,
-                  gen_rtx_POST_DEC (HImode, stack_pointer_rtx));
-  rtx pushword = gen_rtx_MEM (HImode,
-                  gen_rtx_POST_DEC (HImode, stack_pointer_rtx));
   rtx insn;
   
   /* Init cfun->machine.  */
@@ -575,46 +599,34 @@ expand_prologue (void)
 
   if (cfun->machine->is_interrupt || cfun->machine->is_signal)
     {
+      /* Enable interrupts.  */
       if (cfun->machine->is_interrupt)
-        {
-          /* Enable interrupts.  */
-          insn = emit_insn (gen_enable_interrupt ());
-          RTX_FRAME_RELATED_P (insn) = 1;
-        }
+	emit_insn (gen_enable_interrupt ());
 	
       /* Push zero reg.  */
-      insn = emit_move_insn (pushbyte, zero_reg_rtx);
-      RTX_FRAME_RELATED_P (insn) = 1;
-      cfun->machine->stack_usage++;
+      emit_push_byte (ZERO_REGNO, true);
 
       /* Push tmp reg.  */
-      insn = emit_move_insn (pushbyte, tmp_reg_rtx);
-      RTX_FRAME_RELATED_P (insn) = 1;
-      cfun->machine->stack_usage++;
+      emit_push_byte (TMP_REGNO, true);
 
       /* Push SREG.  */
-      insn = emit_move_insn (tmp_reg_rtx, 
-                             gen_rtx_MEM (QImode, GEN_INT (SREG_ADDR)));
-      RTX_FRAME_RELATED_P (insn) = 1;
-      insn = emit_move_insn (pushbyte, tmp_reg_rtx);
-      RTX_FRAME_RELATED_P (insn) = 1;
-      cfun->machine->stack_usage++;
+      /* ??? There's no dwarf2 column reserved for SREG.  */
+      emit_move_insn (tmp_reg_rtx, gen_rtx_MEM (QImode, GEN_INT (SREG_ADDR)));
+      emit_push_byte (TMP_REGNO, false);
 
       /* Push RAMPZ.  */
-      if(AVR_HAVE_RAMPZ 
-         && (TEST_HARD_REG_BIT (set, REG_Z) && TEST_HARD_REG_BIT (set, REG_Z + 1)))
+      /* ??? There's no dwarf2 column reserved for RAMPZ.  */
+      if (AVR_HAVE_RAMPZ 
+          && TEST_HARD_REG_BIT (set, REG_Z)
+          && TEST_HARD_REG_BIT (set, REG_Z + 1))
         {
-          insn = emit_move_insn (tmp_reg_rtx, 
-                                 gen_rtx_MEM (QImode, GEN_INT (RAMPZ_ADDR)));
-          RTX_FRAME_RELATED_P (insn) = 1;
-          insn = emit_move_insn (pushbyte, tmp_reg_rtx);
-          RTX_FRAME_RELATED_P (insn) = 1;
-          cfun->machine->stack_usage++;
+          emit_move_insn (tmp_reg_rtx,
+			  gen_rtx_MEM (QImode, GEN_INT (RAMPZ_ADDR)));
+	  emit_push_byte (TMP_REGNO, false);
         }
 	
       /* Clear zero reg.  */
-      insn = emit_move_insn (zero_reg_rtx, const0_rtx);
-      RTX_FRAME_RELATED_P (insn) = 1;
+      emit_move_insn (zero_reg_rtx, const0_rtx);
 
       /* Prevent any attempt to delete the setting of ZERO_REG!  */
       emit_use (zero_reg_rtx);
@@ -623,37 +635,63 @@ expand_prologue (void)
 		   || (AVR_2_BYTE_PC && live_seq > 6)
 		   || live_seq > 7)) 
     {
-      insn = emit_move_insn (gen_rtx_REG (HImode, REG_X), 
-                             gen_int_mode (size, HImode));
-      RTX_FRAME_RELATED_P (insn) = 1;
+      int first_reg, reg, offset;
 
-      insn = 
-        emit_insn (gen_call_prologue_saves (gen_int_mode (live_seq, HImode),
-					    gen_int_mode (size + live_seq, HImode)));
+      emit_move_insn (gen_rtx_REG (HImode, REG_X), 
+                      gen_int_mode (size, HImode));
+
+      insn = emit_insn (gen_call_prologue_saves
+			(gen_int_mode (live_seq, HImode),
+		         gen_int_mode (size + live_seq, HImode)));
       RTX_FRAME_RELATED_P (insn) = 1;
+
+      /* Describe the effect of the unspec_volatile call to prologue_saves.
+	 Note that this formulation assumes that add_reg_note pushes the
+	 notes to the front.  Thus we build them in the reverse order of
+	 how we want dwarf2out to process them.  */
+
+      /* The function does always set frame_pointer_rtx, but whether that
+	 is going to be permanent in the function is frame_pointer_needed.  */
+      add_reg_note (insn, REG_CFA_ADJUST_CFA,
+		    gen_rtx_SET (VOIDmode,
+				 (frame_pointer_needed
+				  ? frame_pointer_rtx : stack_pointer_rtx),
+				 plus_constant (stack_pointer_rtx,
+						-(size + live_seq))));
+
+      /* Note that live_seq always contains r28+r29, but the other
+	 registers to be saved are all below 18.  */
+      first_reg = 18 - (live_seq - 2);
+
+      for (reg = 29, offset = -live_seq + 1;
+	   reg >= first_reg;
+	   reg = (reg == 28 ? 17 : reg - 1), ++offset)
+	{
+	  rtx m, r;
+
+	  m = gen_rtx_MEM (QImode, plus_constant (stack_pointer_rtx, offset));
+	  r = gen_rtx_REG (QImode, reg);
+	  add_reg_note (insn, REG_CFA_OFFSET, gen_rtx_SET (VOIDmode, m, r));
+	}
+
       cfun->machine->stack_usage += size + live_seq;
     }
   else
     {
       int reg;
       for (reg = 0; reg < 32; ++reg)
-        {
-          if (TEST_HARD_REG_BIT (set, reg))
-            {
-              /* Emit push of register to save.  */
-              insn=emit_move_insn (pushbyte, gen_rtx_REG (QImode, reg));
-              RTX_FRAME_RELATED_P (insn) = 1;
-              cfun->machine->stack_usage++;
-            }
-        }
+        if (TEST_HARD_REG_BIT (set, reg))
+	  emit_push_byte (reg, true);
+
       if (frame_pointer_needed)
         {
 	  if (!(cfun->machine->is_OS_task || cfun->machine->is_OS_main))
 	    {
-              /* Push frame pointer.  */
-	      insn = emit_move_insn (pushword, frame_pointer_rtx);
-              RTX_FRAME_RELATED_P (insn) = 1;
-	      cfun->machine->stack_usage += 2;
+              /* Push frame pointer.  Always be consistent about the
+		 ordering of pushes -- epilogue_restores expects the
+		 register pair to be pushed low byte first.  */
+	      emit_push_byte (REG_Y, true);
+	      emit_push_byte (REG_Y + 1, true);
 	    }
 
           if (!size)
@@ -676,13 +714,12 @@ expand_prologue (void)
               is selected.  */
               rtx myfp;
 	      rtx fp_plus_insns; 
-	      rtx sp_plus_insns = NULL_RTX;
 
               if (AVR_HAVE_8BIT_SP)
                 {
-                  /* The high byte (r29) doesn't change - prefer 'subi' (1 cycle)
-                     over 'sbiw' (2 cycles, same size).  */
-                  myfp = gen_rtx_REG (QImode, REGNO (frame_pointer_rtx));
+                  /* The high byte (r29) doesn't change.  Prefer 'subi'
+		     (1 cycle) over 'sbiw' (2 cycles, same size).  */
+                  myfp = gen_rtx_REG (QImode, FRAME_POINTER_REGNUM);
                 }
               else 
                 {
@@ -693,41 +730,43 @@ expand_prologue (void)
 	      /* Method 1-Adjust frame pointer.  */
 	      start_sequence ();
 
-              insn = emit_move_insn (frame_pointer_rtx, stack_pointer_rtx);
-              RTX_FRAME_RELATED_P (insn) = 1;
+	      /* Normally the dwarf2out frame-related-expr interpreter does
+		 not expect to have the CFA change once the frame pointer is
+		 set up.  Thus we avoid marking the move insn below and
+		 instead indicate that the entire operation is complete after
+		 the frame pointer subtraction is done.  */
 
-              insn = 
-	        emit_move_insn (myfp,
-				gen_rtx_PLUS (GET_MODE(myfp), myfp, 
-					      gen_int_mode (-size, 
-							    GET_MODE(myfp))));
-              RTX_FRAME_RELATED_P (insn) = 1;
+              emit_move_insn (frame_pointer_rtx, stack_pointer_rtx);
 
-	      /* Copy to stack pointer.  */
+              insn = emit_move_insn (myfp, plus_constant (myfp, -size));
+              RTX_FRAME_RELATED_P (insn) = 1;
+	      add_reg_note (insn, REG_CFA_ADJUST_CFA,
+			    gen_rtx_SET (VOIDmode, frame_pointer_rtx,
+					 plus_constant (stack_pointer_rtx,
+							-size)));
+
+	      /* Copy to stack pointer.  Note that since we've already
+		 changed the CFA to the frame pointer this operation
+		 need not be annotated at all.  */
 	      if (AVR_HAVE_8BIT_SP)
 		{
-		  insn = emit_move_insn (stack_pointer_rtx, frame_pointer_rtx);
-		  RTX_FRAME_RELATED_P (insn) = 1;
+		  emit_move_insn (stack_pointer_rtx, frame_pointer_rtx);
 		}
 	      else if (TARGET_NO_INTERRUPTS 
 		       || cfun->machine->is_signal
 		       || cfun->machine->is_OS_main)
 		{
-		  insn = 
-		    emit_insn (gen_movhi_sp_r_irq_off (stack_pointer_rtx, 
-						       frame_pointer_rtx));
-		  RTX_FRAME_RELATED_P (insn) = 1;		
+		  emit_insn (gen_movhi_sp_r_irq_off (stack_pointer_rtx, 
+						     frame_pointer_rtx));
 		}
 	      else if (cfun->machine->is_interrupt)
 		{
-		  insn = emit_insn (gen_movhi_sp_r_irq_on (stack_pointer_rtx, 
-							   frame_pointer_rtx));
-		  RTX_FRAME_RELATED_P (insn) = 1;
+		  emit_insn (gen_movhi_sp_r_irq_on (stack_pointer_rtx, 
+						    frame_pointer_rtx));
 		}
 	      else
 		{
-		  insn = emit_move_insn (stack_pointer_rtx, frame_pointer_rtx);
-		  RTX_FRAME_RELATED_P (insn) = 1;
+		  emit_move_insn (stack_pointer_rtx, frame_pointer_rtx);
 		}
 
 	      fp_plus_insns = get_insns ();
@@ -736,30 +775,30 @@ expand_prologue (void)
 	      /* Method 2-Adjust Stack pointer.  */
               if (size <= 6)
                 {
+		  rtx sp_plus_insns;
+
 		  start_sequence ();
 
-		  insn = 
-		    emit_move_insn (stack_pointer_rtx,
-				    gen_rtx_PLUS (HImode, 
-						  stack_pointer_rtx, 
-						  gen_int_mode (-size, 
-								HImode)));
+	          insn = plus_constant (stack_pointer_rtx, -size);
+		  insn = emit_move_insn (stack_pointer_rtx, insn);
 		  RTX_FRAME_RELATED_P (insn) = 1;
 		  
-		  insn = 
-		    emit_move_insn (frame_pointer_rtx, stack_pointer_rtx);
+		  insn = emit_move_insn (frame_pointer_rtx, stack_pointer_rtx);
 		  RTX_FRAME_RELATED_P (insn) = 1;
 
 		  sp_plus_insns = get_insns ();
 		  end_sequence ();
-                }
 
-              /* Use shortest method.  */
-              if (size <= 6 && (get_sequence_length (sp_plus_insns) 
-				 < get_sequence_length (fp_plus_insns)))
-		emit_insn (sp_plus_insns);
-              else
+		  /* Use shortest method.  */
+		  if (get_sequence_length (sp_plus_insns) 
+		      < get_sequence_length (fp_plus_insns))
+		    emit_insn (sp_plus_insns);
+		  else
+		    emit_insn (fp_plus_insns);
+                }
+	      else
 		emit_insn (fp_plus_insns);
+
 	      cfun->machine->stack_usage += size;
             }
         }
@@ -813,6 +852,20 @@ avr_epilogue_uses (int regno ATTRIBUTE_UNUSED)
   return 0;
 }
 
+/*  Helper for expand_epilogue.  Emit a pop of a byte register.  */
+
+static void
+emit_pop_byte (unsigned regno)
+{
+  rtx mem, reg;
+
+  mem = gen_rtx_PRE_INC (HImode, stack_pointer_rtx);
+  mem = gen_frame_mem (QImode, mem);
+  reg = gen_rtx_REG (QImode, regno);
+
+  emit_insn (gen_rtx_SET (VOIDmode, reg, mem));
+}
+
 /*  Output RTL epilogue.  */
 
 void
@@ -865,13 +918,12 @@ expand_epilogue (void)
               /* Try two methods to adjust stack and select shortest.  */
 	      rtx myfp;
 	      rtx fp_plus_insns;
-	      rtx sp_plus_insns = NULL_RTX;
-	      
+
 	      if (AVR_HAVE_8BIT_SP)
                 {
                   /* The high byte (r29) doesn't change - prefer 'subi' 
                      (1 cycle) over 'sbiw' (2 cycles, same size).  */
-                  myfp = gen_rtx_REG (QImode, REGNO (frame_pointer_rtx));
+                  myfp = gen_rtx_REG (QImode, FRAME_POINTER_REGNUM);
                 }
               else 
                 {
@@ -882,10 +934,7 @@ expand_epilogue (void)
               /* Method 1-Adjust frame pointer.  */
 	      start_sequence ();
 
-	      emit_move_insn (myfp,
-			      gen_rtx_PLUS (GET_MODE (myfp), myfp,
-					    gen_int_mode (size, 
-							  GET_MODE(myfp))));
+	      emit_move_insn (myfp, plus_constant (myfp, size));
 
 	      /* Copy to stack pointer.  */
 	      if (AVR_HAVE_8BIT_SP)
@@ -914,58 +963,63 @@ expand_epilogue (void)
               /* Method 2-Adjust Stack pointer.  */
               if (size <= 5)
                 {
+		  rtx sp_plus_insns;
+
 		  start_sequence ();
 
 		  emit_move_insn (stack_pointer_rtx,
-				  gen_rtx_PLUS (HImode, stack_pointer_rtx,
-						gen_int_mode (size, 
-							      HImode)));
+				  plus_constant (stack_pointer_rtx, size));
 
 		  sp_plus_insns = get_insns ();
 		  end_sequence ();
-                }
 
-              /* Use shortest method.  */
-              if (size <= 5 && (get_sequence_length (sp_plus_insns) 
-				 < get_sequence_length (fp_plus_insns)))
-	      	emit_insn (sp_plus_insns);
-              else
+		  /* Use shortest method.  */
+		  if (get_sequence_length (sp_plus_insns) 
+		      < get_sequence_length (fp_plus_insns))
+		    emit_insn (sp_plus_insns);
+		  else
+		    emit_insn (fp_plus_insns);
+                }
+	      else
 		emit_insn (fp_plus_insns);
             }
 	  if (!(cfun->machine->is_OS_task || cfun->machine->is_OS_main))
 	    {
-              /* Restore previous frame_pointer.  */
-	      emit_insn (gen_pophi (frame_pointer_rtx));
+              /* Restore previous frame_pointer.  See expand_prologue for
+		 rationale for not using pophi.  */
+	      emit_pop_byte (REG_Y + 1);
+	      emit_pop_byte (REG_Y);
 	    }
 	}
+
       /* Restore used registers.  */
       for (reg = 31; reg >= 0; --reg)
-        {
-          if (TEST_HARD_REG_BIT (set, reg))
-              emit_insn (gen_popqi (gen_rtx_REG (QImode, reg)));
-        }
+        if (TEST_HARD_REG_BIT (set, reg))
+          emit_pop_byte (reg);
+
       if (cfun->machine->is_interrupt || cfun->machine->is_signal)
         {
           /* Restore RAMPZ using tmp reg as scratch.  */
-	  if(AVR_HAVE_RAMPZ 
-             && (TEST_HARD_REG_BIT (set, REG_Z) && TEST_HARD_REG_BIT (set, REG_Z + 1)))
+	  if (AVR_HAVE_RAMPZ 
+              && TEST_HARD_REG_BIT (set, REG_Z)
+	      && TEST_HARD_REG_BIT (set, REG_Z + 1))
             {
-	      emit_insn (gen_popqi (tmp_reg_rtx));
-	      emit_move_insn (gen_rtx_MEM(QImode, GEN_INT(RAMPZ_ADDR)), 
+	      emit_pop_byte (TMP_REGNO);
+	      emit_move_insn (gen_rtx_MEM (QImode, GEN_INT (RAMPZ_ADDR)), 
 			      tmp_reg_rtx);
 	    }
 
           /* Restore SREG using tmp reg as scratch.  */
-          emit_insn (gen_popqi (tmp_reg_rtx));
+          emit_pop_byte (TMP_REGNO);
       
-          emit_move_insn (gen_rtx_MEM(QImode, GEN_INT(SREG_ADDR)), 
+          emit_move_insn (gen_rtx_MEM (QImode, GEN_INT (SREG_ADDR)), 
 			  tmp_reg_rtx);
 
           /* Restore tmp REG.  */
-          emit_insn (gen_popqi (tmp_reg_rtx));
+          emit_pop_byte (TMP_REGNO);
 
           /* Restore zero REG.  */
-          emit_insn (gen_popqi (zero_reg_rtx));
+          emit_pop_byte (ZERO_REGNO);
         }
 
       emit_jump_insn (gen_return ());
diff --git a/gcc/config/avr/avr.h b/gcc/config/avr/avr.h
index 9a71078..ee36daf 100644
--- a/gcc/config/avr/avr.h
+++ b/gcc/config/avr/avr.h
@@ -351,9 +351,6 @@ enum reg_class {
 
 #define STATIC_CHAIN_REGNUM 2
 
-/* Offset from the frame pointer register value to the top of the stack.  */
-#define FRAME_POINTER_CFA_OFFSET(FNDECL) 0
-
 #define ELIMINABLE_REGS {					\
       {ARG_POINTER_REGNUM, FRAME_POINTER_REGNUM},		\
 	{FRAME_POINTER_REGNUM, STACK_POINTER_REGNUM}		\
@@ -809,6 +806,13 @@ mmcu=*:-mmcu=%*}"
 
 #define OBJECT_FORMAT_ELF
 
+#define INCOMING_RETURN_ADDR_RTX   avr_incoming_return_addr_rtx ()
+#define INCOMING_FRAME_SP_OFFSET   (AVR_3_BYTE_PC ? 3 : 2)
+
+/* The caller's stack pointer value immediately before the call
+   is one byte below the first argument.  */
+#define ARG_POINTER_CFA_OFFSET(FNDECL)  -1
+
 #define HARD_REGNO_RENAME_OK(OLD_REG, NEW_REG) \
   avr_hard_regno_rename_ok (OLD_REG, NEW_REG)
 
diff --git a/gcc/config/avr/avr.md b/gcc/config/avr/avr.md
index f086e80..f4a95d8 100644
--- a/gcc/config/avr/avr.md
+++ b/gcc/config/avr/avr.md
@@ -182,7 +182,7 @@
 
 
 (define_insn "*pushqi"
-  [(set (mem:QI (post_dec (reg:HI REG_SP)))
+  [(set (mem:QI (post_dec:HI (reg:HI REG_SP)))
         (match_operand:QI 0 "reg_or_0_operand" "r,L"))]
   ""
   "@
@@ -190,9 +190,8 @@
 	push __zero_reg__"
   [(set_attr "length" "1,1")])
 
-
 (define_insn "*pushhi"
-  [(set (mem:HI (post_dec (reg:HI REG_SP)))
+  [(set (mem:HI (post_dec:HI (reg:HI REG_SP)))
         (match_operand:HI 0 "reg_or_0_operand" "r,L"))]
   ""
   "@
@@ -201,7 +200,7 @@
   [(set_attr "length" "2,2")])
 
 (define_insn "*pushsi"
-  [(set (mem:SI (post_dec (reg:HI REG_SP)))
+  [(set (mem:SI (post_dec:HI (reg:HI REG_SP)))
         (match_operand:SI 0 "reg_or_0_operand" "r,L"))]
   ""
   "@
@@ -210,7 +209,7 @@
   [(set_attr "length" "4,4")])
 
 (define_insn "*pushsf"
-  [(set (mem:SF (post_dec (reg:HI REG_SP)))
+  [(set (mem:SF (post_dec:HI (reg:HI REG_SP)))
         (match_operand:SF 0 "register_operand" "r"))]
   ""
   "push %D0
@@ -3127,20 +3126,12 @@
 
 (define_insn "popqi"
   [(set (match_operand:QI 0 "register_operand" "=r")
-        (mem:QI (post_inc (reg:HI REG_SP))))]
+        (mem:QI (pre_inc:HI (reg:HI REG_SP))))]
   ""
   "pop %0"
   [(set_attr "cc" "none")
    (set_attr "length" "1")])
 
-(define_insn "pophi"
-  [(set (match_operand:HI 0 "register_operand" "=r")
-        (mem:HI (post_inc (reg:HI REG_SP))))]
-  ""
-  "pop %A0\;pop %B0"
-  [(set_attr "cc" "none")
-   (set_attr "length" "2")])
-
 ;; Enable Interrupts
 (define_insn "enable_interrupt"
   [(unspec [(const_int 0)] UNSPEC_SEI)]
diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c
index fea8209..b5e660c 100644
--- a/gcc/dwarf2out.c
+++ b/gcc/dwarf2out.c
@@ -2240,7 +2240,7 @@ dwarf2out_frame_debug_cfa_restore (rtx reg, const char *label)
 	   cfa.base_offset = -cfa_store.offset
 
   Rule 11:
-  (set (mem ({pre_inc,pre_dec} sp:cfa_store.reg)) <reg>)
+  (set (mem ({pre_inc,pre_dec,post_dec} sp:cfa_store.reg)) <reg>)
   effects: cfa_store.offset += -/+ mode_size(mem)
 	   cfa.offset = cfa_store.offset if cfa.reg == sp
 	   cfa.reg = sp
@@ -2259,7 +2259,7 @@ dwarf2out_frame_debug_cfa_restore (rtx reg, const char *label)
 	   cfa.base_offset = -{cfa_store,cfa_temp}.offset
 
   Rule 14:
-  (set (mem (postinc <reg1>:cfa_temp <const_int>)) <reg2>)
+  (set (mem (post_inc <reg1>:cfa_temp <const_int>)) <reg2>)
   effects: cfa.reg = <reg1>
 	   cfa.base_offset = -cfa_temp.offset
 	   cfa_temp.offset -= mode_size(mem)
@@ -2592,6 +2592,7 @@ dwarf2out_frame_debug_expr (rtx expr, const char *label)
 	  /* Rule 11 */
 	case PRE_INC:
 	case PRE_DEC:
+	case POST_DEC:
 	  offset = GET_MODE_SIZE (GET_MODE (dest));
 	  if (GET_CODE (XEXP (dest, 0)) == PRE_INC)
 	    offset = -offset;
@@ -2616,7 +2617,10 @@ dwarf2out_frame_debug_expr (rtx expr, const char *label)
 	  if (cfa.reg == STACK_POINTER_REGNUM)
 	    cfa.offset = cfa_store.offset;
 
-	  offset = -cfa_store.offset;
+	  if (GET_CODE (XEXP (dest, 0)) == POST_DEC)
+	    offset += -cfa_store.offset;
+	  else
+	    offset = -cfa_store.offset;
 	  break;
 
 	  /* Rule 12 */

Re: [avr] gas support for cfi info

[Anitha Boyapati]

2011/2/18 Richard Henderson <rth@redhat.com>
>
> On 02/17/2011 08:04 AM, Richard Henderson wrote:
> >> The other scenario is - how about functions with signals/interrupts?
> >> The compiler will give an ICE compiling a function as below:
> >>
> >> void my_interrupt_handler() __attribute__ (("interrupt"));
> >
> > It's a bug in the avr backend; the enable_interrupt insn should not be
> > marked as frame-related.  We should fix this separately.
>
> Looking at this closer, it seem that the FRAME_RELATED markers in avr.c
> were sprinkled at random without really knowing what is going on.

This is quite a clean up! I am yet to reach the changes in expand_epilogue.

>  There
> were 2 places where UNSPEC_VOLATILE insns were marked frame related,
> somehow expecting the unwinder to do something magical.
>

Just to be on same page, these are the 2 places:

1. gen_enable_interrupt()
2. gen_call_prologue_saves()


For the latter, can you  explain why adding reg notes is required?

+      add_reg_note (insn, REG_CFA_ADJUST_CFA,
+		    gen_rtx_SET (VOIDmode,
+				 (frame_pointer_needed
+				  ? frame_pointer_rtx : stack_pointer_rtx),
+				 plus_constant (stack_pointer_rtx,
+						-(size + live_seq))));
+

(The comment does say that this is to describe the effect of
UNSPEC_VOLATILE, but how reg notes help?)


> The following cleans all that up.  There are a couple of odd points:
>
>  (1) SREG and RAMPZ cannot be annotated as saved in the frame of an
>      interrupt function without allocating hard register numbers for
>      these registers.  Not to be confused with "real" registers,
>      these need a number in the same way that SP_L and SP_H have
>      register numbers.
>
>      I've simply ignored unwind info for these for now.
>

In addition to the above, there are atleast 4 more registers RAMPX,
RAMPY, RAMPD and EIND. (Actually the support for these registers is in
the form of patches which aren't upstream yet)


>  (2) At present it's possible to use epilogue_restores without
>      having used prologue_saves.  I.e. use epilogue_restores with
>      an inline prologue.  The problem being that the inline prologue
>      uses an HImode push of REG_Y (i.e. r29 first), whereas the code
>      in prologue_saves pushes r28 first and epilogue_restores is
>      written to expect that.
>

Is this is the line being referred to ?

if (frame_pointer_needed)
  {
...
   /* Push frame pointer.  */
insn = emit_move_insn (pushword, frame_pointer_rtx);


On the whole, when the patch is applied, an ICE is longer generated
during the compilation of interrupt and signal functions.  I am yet to
see the changes due to ARG_POINTER_CFA_OFFSET(FNDECL) definition and
the complete unwind info incase of interrupts.

Anitha

Re: [avr] gas support for cfi info

[Richard Henderson]

On 02/22/2011 08:17 AM, Anitha Boyapati wrote:
> Just to be on same page, these are the 2 places:
> 
> 1. gen_enable_interrupt()
> 2. gen_call_prologue_saves()
> 
> 
> For the latter, can you  explain why adding reg notes is required?
> 
> +      add_reg_note (insn, REG_CFA_ADJUST_CFA,
> +		    gen_rtx_SET (VOIDmode,
> +				 (frame_pointer_needed
> +				  ? frame_pointer_rtx : stack_pointer_rtx),
> +				 plus_constant (stack_pointer_rtx,
> +						-(size + live_seq))));
> +
> 
> (The comment does say that this is to describe the effect of
> UNSPEC_VOLATILE, but how reg notes help?)

The external function call represented by the prologue_saves unspec
saves registers to the stack, and allocates stack space.  Both of
these actions are things we want to describe in unwind info.  The
reg notes I added describe all of the actions performed by the
function call.

>>  (2) At present it's possible to use epilogue_restores without
>>      having used prologue_saves.  I.e. use epilogue_restores with
>>      an inline prologue.  The problem being that the inline prologue
>>      uses an HImode push of REG_Y (i.e. r29 first), whereas the code
>>      in prologue_saves pushes r28 first and epilogue_restores is
>>      written to expect that.
>>
> 
> Is this is the line being referred to ?
> 
> if (frame_pointer_needed)
>   {
> ...
>    /* Push frame pointer.  */
> insn = emit_move_insn (pushword, frame_pointer_rtx);

Yes.


r~