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

[Andrew Cagney <ac131313@redhat.com>]

> 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