This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH] add 'rs6000_in_function_epilogue_p()' (Revised)
On Mon, 5 Dec 2005 11:00:51 -0800
Paul Gilliam <pgilliam@us.ibm.com> wrote:
> I made the changes to address Mark's concerns.
You missed adding spaces around at least one of the operators. The one
that I noticed was the `-' in:
+ for (scan pc = pc-PPC INSN SIZE;
Also, regarding the following bit:
+ ?int opcode ?= (insn>>26) & 0x03f;
+ ?int sd ? ? ?= (insn>>21) & 0x01f;
+ ?int a ? ? ? = (insn>>16) & 0x01f;
+ ?/* ?b ? ? ? = (insn>>11) & 0x01f ?*/
+ ?int subcode = (insn>> 1) & 0x3ff;
+ ?/* ?rc ? ? ?= ?insn ? ? ?& 0x001 ?*/
I like the use of indentation to line things up, but our coding
standard is to remove extraneous spaces. (Because that's what GNU
indent would do. Yeah, I agree it's lame...)
Ah, I just noticed that you missed the spaces around the >> operators too.
> But because of the interest this has drawn, I thought I better ask
> again before I commited the patch.
Thanks for asking again. It was indeed useful to reread the other
comments.
Could I ask you to make the following modifications to your patch?
1) Abort a forward scan (and return 0) if any control transfer instruction
other than blr is found.
2) Abort a backward scan (and return 0) if any control transfer instruction
is found.
3) Place a hard limit on the number of instructions scanned both
forwards and backwards. Epilogues are typically not very big and
scanning to the ends of a large function starting at the middle seems
rather wasteful.
Kevin