* [RFA] mips-tdep.c small cleanup
@ 2004-09-01 23:49 Joel Brobecker
2004-09-02 2:25 ` Andrew Cagney
0 siblings, 1 reply; 3+ messages in thread
From: Joel Brobecker @ 2004-09-01 23:49 UTC (permalink / raw)
To: gdb-patches
[-- Attachment #1: Type: text/plain, Size: 661 bytes --]
I removed some dead code, did some reformatting, and also did one little
code factoring, to make the code look prettier. That's actually the last
part of the change that made me choose a RFA rather than a PATCH. It
looks obvious, and tests ok, but I'd appreciate a second pair of eyes
looking at the change too.
The idea is to clean this code up a little bit, so I can see better
how to implement Andrew's suggestion re- inlining.
2004-09-01 Joel Brobecker <brobecker@gnat.com>
* mips-tdep.c (mips_insn32_frame_cache): Remove some dead code.
Minor reformatting. Some code factoring.
tested on mips-irix 6.5.
OK to apply?
Thanks,
--
Joel
[-- Attachment #2: mips32-cleanup.diff --]
[-- Type: text/plain, Size: 4005 bytes --]
--- mips-tdep.c.new_ref 2004-09-01 17:38:33.996866250 -0400
+++ mips-tdep.c 2004-09-01 18:27:47.336020442 -0400
@@ -2118,28 +2118,16 @@ mips_insn32_frame_cache (struct frame_in
/* Bitmasks; set if we have found a save for the register. */
unsigned long gen_save_found = 0;
unsigned long float_save_found = 0;
- int mips16;
- /* If the address is odd, assume this is MIPS16 code. */
addr = PROC_LOW_ADDR (proc_desc);
- mips16 = pc_is_mips16 (addr);
/* Scan through this function's instructions preceding the
current PC, and look for those that save registers. */
while (addr < frame_pc_unwind (next_frame))
{
- if (mips16)
- {
- mips16_decode_reg_save (mips16_fetch_instruction (addr),
- &gen_save_found);
- addr += MIPS16_INSTLEN;
- }
- else
- {
- mips32_decode_reg_save (mips32_fetch_instruction (addr),
- &gen_save_found, &float_save_found);
- addr += MIPS_INSTLEN;
- }
+ mips32_decode_reg_save (mips32_fetch_instruction (addr),
+ &gen_save_found, &float_save_found);
+ addr += MIPS_INSTLEN;
}
gen_mask = gen_save_found;
float_mask = float_save_found;
@@ -2159,48 +2147,20 @@ mips_insn32_frame_cache (struct frame_in
}
}
- /* The MIPS16 entry instruction saves $s0 and $s1 in the reverse
- order of that normally used by gcc. Therefore, we have to fetch
- the first instruction of the function, and if it's an entry
- instruction that saves $s0 or $s1, correct their saved addresses. */
- if (pc_is_mips16 (PROC_LOW_ADDR (proc_desc)))
- {
- ULONGEST inst = mips16_fetch_instruction (PROC_LOW_ADDR (proc_desc));
- if ((inst & 0xf81f) == 0xe809 && (inst & 0x700) != 0x700)
- /* entry */
- {
- int reg;
- int sreg_count = (inst >> 6) & 3;
-
- /* Check if the ra register was pushed on the stack. */
- CORE_ADDR reg_position = (cache->base
- + PROC_REG_OFFSET (proc_desc));
- if (inst & 0x20)
- reg_position -= mips_abi_regsize (gdbarch);
-
- /* Check if the s0 and s1 registers were pushed on the
- stack. */
- /* NOTE: cagney/2004-02-08: Huh? This is doing no such
- check. */
- for (reg = 16; reg < sreg_count + 16; reg++)
- {
- cache->saved_regs[NUM_REGS + reg].addr = reg_position;
- reg_position -= mips_abi_regsize (gdbarch);
- }
- }
- }
-
/* Fill in the offsets for the registers which float_mask says were
saved. */
{
- CORE_ADDR reg_position = (cache->base
- + PROC_FREG_OFFSET (proc_desc));
+ CORE_ADDR reg_position = (cache->base + PROC_FREG_OFFSET (proc_desc));
int ireg;
+
/* Fill in the offsets for the float registers which float_mask
says were saved. */
for (ireg = MIPS_NUMREGS - 1; float_mask; --ireg, float_mask <<= 1)
if (float_mask & 0x80000000)
{
+ const int regno =
+ NUM_REGS + mips_regnum (current_gdbarch)->fp0 + ireg;
+
if (mips_abi_regsize (gdbarch) == 4
&& TARGET_BYTE_ORDER == BFD_ENDIAN_BIG)
{
@@ -2227,15 +2187,14 @@ mips_insn32_frame_cache (struct frame_in
reg_position is decremented each time through the
loop). */
if ((ireg & 1))
- cache->saved_regs[NUM_REGS + mips_regnum (current_gdbarch)->fp0 + ireg]
- .addr = reg_position - mips_abi_regsize (gdbarch);
+ cache->saved_regs[regno].addr =
+ reg_position - mips_abi_regsize (gdbarch);
else
- cache->saved_regs[NUM_REGS + mips_regnum (current_gdbarch)->fp0 + ireg]
- .addr = reg_position + mips_abi_regsize (gdbarch);
+ cache->saved_regs[regno].addr =
+ reg_position + mips_abi_regsize (gdbarch);
}
else
- cache->saved_regs[NUM_REGS + mips_regnum (current_gdbarch)->fp0 + ireg]
- .addr = reg_position;
+ cache->saved_regs[regno].addr = reg_position;
reg_position -= mips_abi_regsize (gdbarch);
}
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [RFA] mips-tdep.c small cleanup
2004-09-01 23:49 [RFA] mips-tdep.c small cleanup Joel Brobecker
@ 2004-09-02 2:25 ` Andrew Cagney
2004-09-02 22:58 ` Joel Brobecker
0 siblings, 1 reply; 3+ messages in thread
From: Andrew Cagney @ 2004-09-02 2:25 UTC (permalink / raw)
To: Joel Brobecker; +Cc: gdb-patches
> I removed some dead code, did some reformatting, and also did one little
> code factoring, to make the code look prettier. That's actually the last
> part of the change that made me choose a RFA rather than a PATCH. It
> looks obvious, and tests ok, but I'd appreciate a second pair of eyes
> looking at the change too.
>
> The idea is to clean this code up a little bit, so I can see better
> how to implement Andrew's suggestion re- inlining.
>
> 2004-09-01 Joel Brobecker <brobecker@gnat.com>
>
> * mips-tdep.c (mips_insn32_frame_cache): Remove some dead code.
> Minor reformatting. Some code factoring.
>
> tested on mips-irix 6.5.
> OK to apply?
Yep!
Andrew
> --- mips-tdep.c.new_ref 2004-09-01 17:38:33.996866250 -0400
> +++ mips-tdep.c 2004-09-01 18:27:47.336020442 -0400
> @@ -2118,28 +2118,16 @@ mips_insn32_frame_cache (struct frame_in
> /* Bitmasks; set if we have found a save for the register. */
> unsigned long gen_save_found = 0;
> unsigned long float_save_found = 0;
> - int mips16;
>
> - /* If the address is odd, assume this is MIPS16 code. */
> addr = PROC_LOW_ADDR (proc_desc);
> - mips16 = pc_is_mips16 (addr);
>
> /* Scan through this function's instructions preceding the
> current PC, and look for those that save registers. */
> while (addr < frame_pc_unwind (next_frame))
> {
> - if (mips16)
> - {
> - mips16_decode_reg_save (mips16_fetch_instruction (addr),
> - &gen_save_found);
> - addr += MIPS16_INSTLEN;
> - }
> - else
> - {
> - mips32_decode_reg_save (mips32_fetch_instruction (addr),
> - &gen_save_found, &float_save_found);
> - addr += MIPS_INSTLEN;
> - }
> + mips32_decode_reg_save (mips32_fetch_instruction (addr),
> + &gen_save_found, &float_save_found);
> + addr += MIPS_INSTLEN;
> }
> gen_mask = gen_save_found;
> float_mask = float_save_found;
> @@ -2159,48 +2147,20 @@ mips_insn32_frame_cache (struct frame_in
> }
> }
>
> - /* The MIPS16 entry instruction saves $s0 and $s1 in the reverse
> - order of that normally used by gcc. Therefore, we have to fetch
> - the first instruction of the function, and if it's an entry
> - instruction that saves $s0 or $s1, correct their saved addresses. */
> - if (pc_is_mips16 (PROC_LOW_ADDR (proc_desc)))
> - {
> - ULONGEST inst = mips16_fetch_instruction (PROC_LOW_ADDR (proc_desc));
> - if ((inst & 0xf81f) == 0xe809 && (inst & 0x700) != 0x700)
> - /* entry */
> - {
> - int reg;
> - int sreg_count = (inst >> 6) & 3;
> -
> - /* Check if the ra register was pushed on the stack. */
> - CORE_ADDR reg_position = (cache->base
> - + PROC_REG_OFFSET (proc_desc));
> - if (inst & 0x20)
> - reg_position -= mips_abi_regsize (gdbarch);
> -
> - /* Check if the s0 and s1 registers were pushed on the
> - stack. */
> - /* NOTE: cagney/2004-02-08: Huh? This is doing no such
> - check. */
> - for (reg = 16; reg < sreg_count + 16; reg++)
> - {
> - cache->saved_regs[NUM_REGS + reg].addr = reg_position;
> - reg_position -= mips_abi_regsize (gdbarch);
> - }
> - }
> - }
> -
> /* Fill in the offsets for the registers which float_mask says were
> saved. */
> {
> - CORE_ADDR reg_position = (cache->base
> - + PROC_FREG_OFFSET (proc_desc));
> + CORE_ADDR reg_position = (cache->base + PROC_FREG_OFFSET (proc_desc));
> int ireg;
> +
> /* Fill in the offsets for the float registers which float_mask
> says were saved. */
> for (ireg = MIPS_NUMREGS - 1; float_mask; --ireg, float_mask <<= 1)
> if (float_mask & 0x80000000)
> {
> + const int regno =
> + NUM_REGS + mips_regnum (current_gdbarch)->fp0 + ireg;
> +
> if (mips_abi_regsize (gdbarch) == 4
> && TARGET_BYTE_ORDER == BFD_ENDIAN_BIG)
> {
> @@ -2227,15 +2187,14 @@ mips_insn32_frame_cache (struct frame_in
> reg_position is decremented each time through the
> loop). */
> if ((ireg & 1))
> - cache->saved_regs[NUM_REGS + mips_regnum (current_gdbarch)->fp0 + ireg]
> - .addr = reg_position - mips_abi_regsize (gdbarch);
> + cache->saved_regs[regno].addr =
> + reg_position - mips_abi_regsize (gdbarch);
> else
> - cache->saved_regs[NUM_REGS + mips_regnum (current_gdbarch)->fp0 + ireg]
> - .addr = reg_position + mips_abi_regsize (gdbarch);
> + cache->saved_regs[regno].addr =
> + reg_position + mips_abi_regsize (gdbarch);
> }
> else
> - cache->saved_regs[NUM_REGS + mips_regnum (current_gdbarch)->fp0 + ireg]
> - .addr = reg_position;
> + cache->saved_regs[regno].addr = reg_position;
> reg_position -= mips_abi_regsize (gdbarch);
> }
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [RFA] mips-tdep.c small cleanup
2004-09-02 2:25 ` Andrew Cagney
@ 2004-09-02 22:58 ` Joel Brobecker
0 siblings, 0 replies; 3+ messages in thread
From: Joel Brobecker @ 2004-09-02 22:58 UTC (permalink / raw)
To: Andrew Cagney; +Cc: gdb-patches
> >2004-09-01 Joel Brobecker <brobecker@gnat.com>
> >
> > * mips-tdep.c (mips_insn32_frame_cache): Remove some dead code.
> > Minor reformatting. Some code factoring.
> >
> >tested on mips-irix 6.5.
> >OK to apply?
>
> Yep!
Thanks. Checked in.
--
Joel
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2004-09-02 22:58 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2004-09-01 23:49 [RFA] mips-tdep.c small cleanup Joel Brobecker
2004-09-02 2:25 ` Andrew Cagney
2004-09-02 22:58 ` Joel Brobecker
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox