This is the mail archive of the gdb-patches@sources.redhat.com mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [patch rfc] Add NUM_REGS pseudo regs to MIPS


On Jun 16, 11:35am, Andrew Cagney wrote:


Per my last post to an old thread. This adds NUM_REGS pseudo registers to the MIPS. These pseudo registers, unlike their raw counterparts are `sane'. They have sensible sizes, offsets, types, ...

The intent here is to put some distance between the MIPS's messed up raw register buffer and the ABI registers. As it stands, it doesn't get save/restore right (it works but not by using the ABI registers). That can follow.

Tested on mips-elf.

thoughts?


In light of the recent o32 ABI discussion, I believe the approach
that I used is correct for both the ABI / debug info case as well
as the cli registers case.  I think you should reconsider my patch
before proceeding.

I'm constantly refering back to your original WIP patch. My current take on those discussions, though, is that this part of that change:


@@ -1581,11 +1680,25 @@ mips_find_saved_regs (struct frame_info
   for (ireg = MIPS_NUMREGS - 1; float_mask; --ireg, float_mask <<= 1)
     if (float_mask & 0x80000000)
       {
-	get_frame_saved_regs (fci)[FP0_REGNUM + ireg] = reg_position;
+	get_frame_saved_regs (fci)[raw_regnums->fp0_regnum + ireg]
+	  = reg_position;
+
+	/* Now take care of the cooked register number.  */
+	if (!FP_REGISTER_DOUBLE && MIPS_FPU_TYPE == MIPS_FPU_DOUBLE)
+	  {
+	    if ((ireg & 1) == 0)
+	      get_frame_saved_regs (fci)[cooked_regnums->fp0_regnum + ireg / 2]
+		= reg_position;
+	  }
+	else
+	  get_frame_saved_regs (fci)[cooked_regnums->fp0_regnum + ireg]
+	    = reg_position;
+
 	reg_position -= MIPS_SAVED_REGSIZE;
       }

which lies at the core, is wrong. As I wrote:

Ref: mips_find_saved_regs /float_mask/ at the end of the function.
I believe that the debug info indicates that $f20/$f21 were both saved.  The code comes with the comment:

    /* Apparently, the freg_offset gives the offset to the first 64
       bit saved.

       When the ABI specifies 64 bit saved registers, the FREG_OFFSET
       designates the first saved 64 bit register.

       When the ABI specifies 32 bit saved registers, the ``64 bit
       saved DOUBLE'' consists of two adjacent 32 bit registers, Hence
       FREG_OFFSET, designates the address of the lower register of
       the register pair.  Adjust the offset so that it designates the
       upper register of the pair -- i.e., the address of the first
       saved 32 bit register.  */

Now, from the thread so far, it is clear that the comment is only partially correct. It should indicate that:

On a big endian 32 bit ABI, the compiler spills floating-point registers as a pair and as a floating-point double. Because the target is big-endian, this leads to the register pair being stored in reverse order vis: $fN ||| $fN+1 are stored as $fN+1 and then $fN.

The code doesn't do that, it gets the address of $fN correct, but $fN+1 is 8 bytes out. Outch!

Given o32, GDB needs to track the location of the individual 32 bit floating point registers and not 64 bit FP pairs. By doing that, the code (mips_register_to_value):

  frame_read_register (frame, regnum + 0, (char *) to + 4);
  frame_read_register (frame, regnum + 1, (char *) to + 0);

is able to correctly construct a double value for any frame.

Committing it will make, the already impossible task of getting the MIPS frame code using the unwinders, even harder.


Andrew



Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]