Re: RFA: ia64 tdep patch

["J. Johnston" <jjohnstn@redhat.com>]

Kevin Buettner wrote:
> On Oct 21,  7:03pm, J. Johnston wrote:
> 
> 
>>You're looking at my old ChangeLog.
> 
> 
> I've looked at your most recent patch, but do not see the new ChangeLog
> entry.  Would you mind posting it?
> 

2003-10-20  Jeff Johnston  <jjohnstn@redhat.com>

     * ia64-tdep.c: (ia64_frame_cache): Add new prev_cfm field.
     (ia64_alloc_frame_cache):  Initialize new prev_cfm field to 0.
     (floatformat_valid): New static routine.
     (floatformat_ia64_ext): Add name field and set up is_valid routine
     to floatformat_valid().
     (examine_prologue):  For the previous cfm, use frame_unwind_register()
     if the cfm is not stored in a register-stack register.  Save the
     previous cfm value in the prev_cfm field.  Add debug output.
     (ia64_frame_this_id): Use frame_id_build_special() to also register
     the bsp.  Add debug output.
     (ia64_sigtramp_frame_this_id): Ditto.
     (ia64_frame_prev_register):  Look at cache saved_regs for a few more
     registers and also add some checks for framelessness before accepting
     current register values for fields such as return address.  For cfm,
     use the cached prev_cfm field if available.  Add debug output.
     (ia64_sigtramp_frame_init_saved_regs): Bump up base by 16 to get
     sp needed for calling lower level
     ia64_linux_sigcontext_register_address().  Also save the
     bsp and sp address as part of initialization.
     (ia64_sigtramp_frame_cache): Hard-code stack size as it can't be
     calculated.  Cache the bsp and cfm values.
     (ia64_sigtramp_frame_prev_register): Flesh this routine out instead of
     using ia64_frame_prev_register().  The saved values for bsp and sp can
     be taken from the cache.  Add debug output.
     (ia64_push_dummy_call): Use frame_id_build_special() to also register
     the bsp.


> 
>>>The code (below) in ia64_sigtramp_frame_prev_register() which computes
>>>PSR doesn't look right to me.  Could you check it?  (If it is right,
>>>please explain it...)
>>
>>I'll explain my logic.  As you know, the VRAP address is the return address. 
>>AFAICT by reading the ABI and insn set, there is no information about what the 
>>return address is set to when the branch is in slot 0 or 1 (i.e. is the return 
>>address the next bundle or the next slot?).  The ip register isn't supposed to 
>>contain the slot number;  it is encoded in the PSR register.  When gdb gets the 
>>pc value, it forms it by unwinding the PSR and IP registers - getting the slot 
>>number from the PSR and the IP register address to form a virtual pc address.  I 
>>did not want to get the slot number wrong if it was encoded in the return 
>>address so this is why I masked it off above.  The PSR register is only used by 
>>gdb in unwinding the pc.
> 
> 
> Thanks for the explanation.  Could you please add a brief comment to the
> code.
> 

Will do.

> 
>>>Regarding this hunk of code in ia64_sigtramp_frame_prev_register()...
>>>
>>>+ else if ((regnum >= IA64_GR32_REGNUM && regnum <= IA64_GR127_REGNUM) ||
>>>+	   (regnum >= V32_REGNUM && regnum <= V127_REGNUM))
>>>+    {
>>>+      CORE_ADDR addr = 0;
>>>+      if (regnum >= V32_REGNUM)
>>>+	regnum = IA64_GR32_REGNUM + (regnum - V32_REGNUM);
>>>+      addr = cache->saved_regs[regnum];
>>>+      if (addr != 0)
>>>+	{
>>>+	  *lvalp = lval_memory;
>>>+	  *addrp = addr;
>>>+	  read_memory (addr, valuep, register_size (current_gdbarch, regnum));
>>>+	}
>>>+    }
>>>
>>>Could you add a comment explaining why the normal method of computing
>>>V32 (via ia64_pseudo_register_read()) is inadequate?
>>
>>I don't know.  I had this for safety reasons already in the
>>ia64_frame_prev_register() because I didn't know if it might be
>>called with the pseudo register number or not.  This code was
>>copied.  Should it be removed in both places?
> 
> 
> I don't know.  I've been studying the code and am wondering why the
> V32 ... V127 pseudo regs were introduced at all.  Could you remind
> me of the reason?
>

They are needed because r32 to r127 are not accessible via the PTRACE interface. 
  They are accessed via the bsp.  Without flagging them as pseudo-registers, the 
regcache code returns 0 for all these registers.

I tell the dwarf to convert references to r32-r127 to be V32-V127 in 
ia64_dwarf_reg_to_regnum().  I would guess that a parameter reference in a 
previous frame would cause this conversion to occur.

> 
>>>Hmm... doesn't this hunk of code also need to be concerned with
>>>register renames?  (I.e, the rotating register stuff...)  I'm
>>>wondering why the floating point registers need it, but the
>>>GRs don't.
>>>
>>
>>This code was copied from ia64_frame_prev_register() as it used to be called to 
>>do the underlying work.
>>
>>The stuff at the end of examine_prologue() handles rotating GRs for
>>the normal case but doesn't for floating point registers.  I would
>>doubt very much that the signal trampoline uses rotating registers
>>so I probably should remove it for the floating-point case.
> 
> 
> I think you're probably right.
> 
> It'd be nice though if we could arrange for the code which handles
> rotating registers for floating point and general regs to appear
> in the same function.  (This can happen in a different patch though.)
> 
> Kevin
>