diff mbox

xtensa: fix access to the last pseudo register

Message ID 1431904329-13965-1-git-send-email-jcmvbkbc@gmail.com
State New, archived
Headers

Commit Message

Max Filippov May 17, 2015, 11:12 p.m. UTC 
  Currently access to the last pseudo register is aliased to a1. This is
done by little snippets in the beginning of xtensa_pseudo_register_read
and xtensa_pseudo_register_write that used to do such aliasing for FP
register since bdb4c075a29dd086f0868b394b488b1c94666be6, but then
FP_ALIAS was expanded into gdbarch_num_regs (current_gdbarch) +
gdbarch_num_pseudo_regs (current_gdbarch) (one register past the last
pseudo register) in 304fe2552d6e0821e8fdb7575f8e7ba6607a076d, which
then was changed to the last pseudo register in
94a0e877111421d300d26b858bd3a0a27078d1e8.

Drop these snippets.

2015-05-18  Max Filippov  <jcmvbkbc@gmail.com>
gdb/
	* xtensa-tdep.c (xtensa_pseudo_register_read)
	(xtensa_pseudo_register_write): Don't alias last pseudo register
	to a1.
---
 gdb/xtensa-tdep.c | 8 --------
 1 file changed, 8 deletions(-)

Comments

Max Filippov May 27, 2015, 2:11 a.m. UTC | #1 
On Mon, May 18, 2015 at 2:12 AM, Max Filippov <jcmvbkbc@gmail.com> wrote:
> Currently access to the last pseudo register is aliased to a1. This is
> done by little snippets in the beginning of xtensa_pseudo_register_read
> and xtensa_pseudo_register_write that used to do such aliasing for FP
> register since bdb4c075a29dd086f0868b394b488b1c94666be6, but then
> FP_ALIAS was expanded into gdbarch_num_regs (current_gdbarch) +
> gdbarch_num_pseudo_regs (current_gdbarch) (one register past the last
> pseudo register) in 304fe2552d6e0821e8fdb7575f8e7ba6607a076d, which
> then was changed to the last pseudo register in
> 94a0e877111421d300d26b858bd3a0a27078d1e8.
>
> Drop these snippets.
>
> 2015-05-18  Max Filippov  <jcmvbkbc@gmail.com>
> gdb/
>         * xtensa-tdep.c (xtensa_pseudo_register_read)
>         (xtensa_pseudo_register_write): Don't alias last pseudo register
>         to a1.
> ---
>  gdb/xtensa-tdep.c | 8 --------
>  1 file changed, 8 deletions(-)

Ping?
Pedro Alves May 27, 2015, 10:16 a.m. UTC | #2 
On 05/18/2015 12:12 AM, Max Filippov wrote:
> Currently access to the last pseudo register is aliased to a1. This is
> done by little snippets in the beginning of xtensa_pseudo_register_read
> and xtensa_pseudo_register_write that used to do such aliasing for FP
> register since bdb4c075a29dd086f0868b394b488b1c94666be6, but then
> FP_ALIAS was expanded into gdbarch_num_regs (current_gdbarch) +
> gdbarch_num_pseudo_regs (current_gdbarch) (one register past the last
> pseudo register) in 304fe2552d6e0821e8fdb7575f8e7ba6607a076d, which
> then was changed to the last pseudo register in
> 94a0e877111421d300d26b858bd3a0a27078d1e8.

IIUC, the original intention was for FP to alias a1, and then through
that series of patches (part of old current_gdbarch elimination)
we ended up aliasing the wrong register.  Instead of fixing the
aliasing, you're just removing it altogether.  Correct?

OK.

Thanks,
Pedro Alves
Max Filippov May 27, 2015, 10:55 a.m. UTC | #3 
On Wed, May 27, 2015 at 1:16 PM, Pedro Alves <palves@redhat.com> wrote:
> On 05/18/2015 12:12 AM, Max Filippov wrote:
>> Currently access to the last pseudo register is aliased to a1. This is
>> done by little snippets in the beginning of xtensa_pseudo_register_read
>> and xtensa_pseudo_register_write that used to do such aliasing for FP
>> register since bdb4c075a29dd086f0868b394b488b1c94666be6, but then
>> FP_ALIAS was expanded into gdbarch_num_regs (current_gdbarch) +
>> gdbarch_num_pseudo_regs (current_gdbarch) (one register past the last
>> pseudo register) in 304fe2552d6e0821e8fdb7575f8e7ba6607a076d, which
>> then was changed to the last pseudo register in
>> 94a0e877111421d300d26b858bd3a0a27078d1e8.
>
> IIUC, the original intention was for FP to alias a1, and then through
> that series of patches (part of old current_gdbarch elimination)
> we ended up aliasing the wrong register.  Instead of fixing the
> aliasing, you're just removing it altogether.  Correct?

I'm removing that aliasing to expose the last pseudoregister.
After that patch fp pseudo register still exists and it is a read only alias
for a1. I don't think that's right, because normally frame pointer is a7 or
a15, depending on ABI, but that's definitely a separate question.
Marc Gauthier May 27, 2015, 3:12 p.m. UTC | #4 
Max Filippov wrote:
[...]
> I'm removing that aliasing to expose the last pseudoregister.

> After that patch fp pseudo register still exists and it is a read only

> alias

> for a1. I don't think that's right, because normally frame pointer is a7

> or

> a15, depending on ABI, but that's definitely a separate question.


If the compiler allocated a frame pointer for the function, it is
a7 or a15 according to ABI, correct.  If no separate pointer is
allocated (usually when no variable stack allocation such as alloca
or similar is used in the function), I believe a1 is effectively
both the stack pointer and the frame pointer.

-Marc
Pedro Alves May 29, 2015, 9:40 a.m. UTC | #5 
On 05/27/2015 11:55 AM, Max Filippov wrote:
> On Wed, May 27, 2015 at 1:16 PM, Pedro Alves <palves@redhat.com> wrote:
>> On 05/18/2015 12:12 AM, Max Filippov wrote:
>>> Currently access to the last pseudo register is aliased to a1. This is
>>> done by little snippets in the beginning of xtensa_pseudo_register_read
>>> and xtensa_pseudo_register_write that used to do such aliasing for FP
>>> register since bdb4c075a29dd086f0868b394b488b1c94666be6, but then
>>> FP_ALIAS was expanded into gdbarch_num_regs (current_gdbarch) +
>>> gdbarch_num_pseudo_regs (current_gdbarch) (one register past the last
>>> pseudo register) in 304fe2552d6e0821e8fdb7575f8e7ba6607a076d, which
>>> then was changed to the last pseudo register in
>>> 94a0e877111421d300d26b858bd3a0a27078d1e8.
>>
>> IIUC, the original intention was for FP to alias a1, and then through
>> that series of patches (part of old current_gdbarch elimination)
>> we ended up aliasing the wrong register.  Instead of fixing the
>> aliasing, you're just removing it altogether.  Correct?
> 
> I'm removing that aliasing to expose the last pseudoregister.
> After that patch fp pseudo register still exists and it is a read only alias
> for a1. I don't think that's right, because normally frame pointer is a7 or
> a15, depending on ABI, but that's definitely a separate question.

Ah, ok.  FAOD, patch is still OK.  :-)

Thanks,
Pedro Alves
Max Filippov May 29, 2015, 10:36 a.m. UTC | #6 
On Fri, May 29, 2015 at 12:40 PM, Pedro Alves <palves@redhat.com> wrote:
> On 05/27/2015 11:55 AM, Max Filippov wrote:
>> On Wed, May 27, 2015 at 1:16 PM, Pedro Alves <palves@redhat.com> wrote:
>>> On 05/18/2015 12:12 AM, Max Filippov wrote:
>>>> Currently access to the last pseudo register is aliased to a1. This is
>>>> done by little snippets in the beginning of xtensa_pseudo_register_read
>>>> and xtensa_pseudo_register_write that used to do such aliasing for FP
>>>> register since bdb4c075a29dd086f0868b394b488b1c94666be6, but then
>>>> FP_ALIAS was expanded into gdbarch_num_regs (current_gdbarch) +
>>>> gdbarch_num_pseudo_regs (current_gdbarch) (one register past the last
>>>> pseudo register) in 304fe2552d6e0821e8fdb7575f8e7ba6607a076d, which
>>>> then was changed to the last pseudo register in
>>>> 94a0e877111421d300d26b858bd3a0a27078d1e8.
>>>
>>> IIUC, the original intention was for FP to alias a1, and then through
>>> that series of patches (part of old current_gdbarch elimination)
>>> we ended up aliasing the wrong register.  Instead of fixing the
>>> aliasing, you're just removing it altogether.  Correct?
>>
>> I'm removing that aliasing to expose the last pseudoregister.
>> After that patch fp pseudo register still exists and it is a read only alias
>> for a1. I don't think that's right, because normally frame pointer is a7 or
>> a15, depending on ABI, but that's definitely a separate question.
>
> Ah, ok.  FAOD, patch is still OK.  :-)

Thanks! Checked in.

-- Max
diff mbox

Patch

diff --git a/gdb/xtensa-tdep.c b/gdb/xtensa-tdep.c
index a66d00a..55e7d98 100644
--- a/gdb/xtensa-tdep.c
+++ b/gdb/xtensa-tdep.c
@@ -553,10 +553,6 @@  xtensa_pseudo_register_read (struct gdbarch *gdbarch,
   DEBUGTRACE ("xtensa_pseudo_register_read (... regnum = %d (%s) ...)\n",
 	      regnum, xtensa_register_name (gdbarch, regnum));
 
-  if (regnum == gdbarch_num_regs (gdbarch)
-		+ gdbarch_num_pseudo_regs (gdbarch) - 1)
-     regnum = gdbarch_tdep (gdbarch)->a0_base + 1;
-
   /* Read aliases a0..a15, if this is a Windowed ABI.  */
   if (gdbarch_tdep (gdbarch)->isa_use_windowed_registers
       && (regnum >= gdbarch_tdep (gdbarch)->a0_base)
@@ -653,10 +649,6 @@  xtensa_pseudo_register_write (struct gdbarch *gdbarch,
   DEBUGTRACE ("xtensa_pseudo_register_write (... regnum = %d (%s) ...)\n",
 	      regnum, xtensa_register_name (gdbarch, regnum));
 
-  if (regnum == gdbarch_num_regs (gdbarch)
-		+ gdbarch_num_pseudo_regs (gdbarch) -1)
-     regnum = gdbarch_tdep (gdbarch)->a0_base + 1;
-
   /* Renumber register, if aliase a0..a15 on Windowed ABI.  */
   if (gdbarch_tdep (gdbarch)->isa_use_windowed_registers
       && (regnum >= gdbarch_tdep (gdbarch)->a0_base)