[v5] gdb/hppa-tdep.c: Fix logical working flow issues and check additional store instructions
Commit Message
Original working flow has several issues:
- typo issue: "(inst >> 26) == 0x1f && ..." for checking 'stw(m)'.
- compiler warning: "(inst >> 6) & 0xf) == 0x9" for checking 'sth'.
- "(inst >> 6) == 0xa" needs to be "((inst >> 6) & 0xf) == 0xa".
And also need check additional store instructions:
- For absolute memory: 'stby', 'stdby'.
- For unaligned: 'stwa', 'stda'.
The original code also can be improved:
- Remove redundant double check "(inst >> 26) == 0x1b" for 'stwm'.
- Use 2 'switch' statements instead of all 'if' statements.
2014-12-19 Chen Gang <gang.chen.5i5j@gmail.com>
* hppa-tdep.c (inst_saves_gr): Fix logical working flow issues
and check additional store instructions.
---
gdb/hppa-tdep.c | 121 ++++++++++++++++++++++++++++++++++++++++++++------------
1 file changed, 95 insertions(+), 26 deletions(-)
Comments
> 2014-12-19 Chen Gang <gang.chen.5i5j@gmail.com>
>
> * hppa-tdep.c (inst_saves_gr): Fix logical working flow issues
> and check additional store instructions.
Thank you. The patch looks good to me, but I don't seem to find
a copyright assignment in file for you at the FSF. Do you happen
to have one? Your contribution is over the 15 lines limit, and
can therefore be considered legally significant, which is why
we need you to have a copyright assignment on file to transfer
the copyright of your changes over to the FSF before we can accept
this patch.
Let me know if you need me to send you the forms to get you started.
It can take a while to complete.
On 12/21/2014 01:26 AM, Joel Brobecker wrote:
>> 2014-12-19 Chen Gang <gang.chen.5i5j@gmail.com>
>>
>> * hppa-tdep.c (inst_saves_gr): Fix logical working flow issues
>> and check additional store instructions.
>
> Thank you. The patch looks good to me, but I don't seem to find
> a copyright assignment in file for you at the FSF. Do you happen
> to have one? Your contribution is over the 15 lines limit, and
> can therefore be considered legally significant, which is why
> we need you to have a copyright assignment on file to transfer
> the copyright of your changes over to the FSF before we can accept
> this patch.
>
OK, thanks, I shall follow the related working flow.
For me, I don't quite care about whether my contribute is appreciated,
for what I have done is only for contribution back to open source (it is
what I should do -- as a true programmer).
I mainly care about what I have done should have no negative effect with
other members, and don't waste other members' time resources.
> Let me know if you need me to send you the forms to get you started.
> It can take a while to complete.
>
Yeah, please help send the forms to me.
Hello Maintainers:
If no any additional replies within this month (2015-01-31), I shall
"git push" this patch.
Thanks again for Joel Brokecker's patient help for the related working
flow (originally, I almost knew nothing about the assignment working
flow).
And sorry, gmail is broken again, in China, I have to use my another
email again (I tried to reply by gmail via web site, but gdb-patches can
not accept it, because it is not pure text format).
Thanks.
On 12/21/14 01:26, Joel Brobecker wrote:
>> 2014-12-19 Chen Gang <gang.chen.5i5j@gmail.com>
>>
>> * hppa-tdep.c (inst_saves_gr): Fix logical working flow issues
>> and check additional store instructions.
>
> Thank you. The patch looks good to me, but I don't seem to find
> a copyright assignment in file for you at the FSF. Do you happen
> to have one? Your contribution is over the 15 lines limit, and
> can therefore be considered legally significant, which is why
> we need you to have a copyright assignment on file to transfer
> the copyright of your changes over to the FSF before we can accept
> this patch.
>
> Let me know if you need me to send you the forms to get you started.
> It can take a while to complete.
>
> If no any additional replies within this month (2015-01-31), I shall
> "git push" this patch.
You do not need to wait. The patch has been approved, so go ahead
and push now. If there are any issues, we can handle them when
they get reported.
On 01/21/2015 06:56 PM, Joel Brobecker wrote:
>> If no any additional replies within this month (2015-01-31), I shall
>> "git push" this patch.
>
> You do not need to wait. The patch has been approved, so go ahead
> and push now. If there are any issues, we can handle them when
> they get reported.
>
OK, thanks. finish "git push" to master based on current version, please
check.
At present, the related code are changed a little (fix gcc-5 warning).
So need change the patch a little, but have no effect with the final
applied code.
Thanks.
> At present, the related code are changed a little (fix gcc-5 warning).
> So need change the patch a little, but have no effect with the final
> applied code.
OK. The expectation, when you push something that's different from
what you had reviewed is to send the final version of the patch on
this list. I find this is a little less important, now that we have
diffs in gdb-cvs emails, as well as now that we use git, but it's
part of our procedures; so would you mind please sending a copy here?
Thank you,
On 1/23/15 15:31, Joel Brobecker wrote:
>> At present, the related code are changed a little (fix gcc-5 warning).
>> So need change the patch a little, but have no effect with the final
>> applied code.
>
> OK. The expectation, when you push something that's different from
> what you had reviewed is to send the final version of the patch on
> this list. I find this is a little less important, now that we have
> diffs in gdb-cvs emails, as well as now that we use git, but it's
> part of our procedures; so would you mind please sending a copy here?
>
Oh, sorry, I shall send the copy soon. Next, I should notice about it.
Thanks.
@@ -1376,37 +1376,106 @@ is_branch (unsigned long inst)
}
/* Return the register number for a GR which is saved by INST or
- zero it INST does not save a GR. */
+ zero if INST does not save a GR.
-static int
-inst_saves_gr (unsigned long inst)
-{
- /* Does it look like a stw? */
- if ((inst >> 26) == 0x1a || (inst >> 26) == 0x1b
- || (inst >> 26) == 0x1f
- || ((inst >> 26) == 0x1f
- && ((inst >> 6) == 0xa)))
- return hppa_extract_5R_store (inst);
+ Referenced from:
- /* Does it look like a std? */
- if ((inst >> 26) == 0x1c
- || ((inst >> 26) == 0x03
- && ((inst >> 6) & 0xf) == 0xb))
- return hppa_extract_5R_store (inst);
+ parisc 1.1:
+ https://parisc.wiki.kernel.org/images-parisc/6/68/Pa11_acd.pdf
- /* Does it look like a stwm? GCC & HPC may use this in prologues. */
- if ((inst >> 26) == 0x1b)
- return hppa_extract_5R_store (inst);
+ parisc 2.0:
+ https://parisc.wiki.kernel.org/images-parisc/7/73/Parisc2.0.pdf
- /* Does it look like sth or stb? HPC versions 9.0 and later use these
- too. */
- if ((inst >> 26) == 0x19 || (inst >> 26) == 0x18
- || ((inst >> 26) == 0x3
- && (((inst >> 6) & 0xf) == 0x8
- || (inst >> 6) & 0xf) == 0x9))
- return hppa_extract_5R_store (inst);
+ According to Table 6-5 of Chapter 6 (Memory Reference Instructions)
+ on page 106 in parisc 2.0, all instructions for storing values from
+ the general registers are:
- return 0;
+ Store: stb, sth, stw, std (according to Chapter 7, they
+ are only in both "inst >> 26" and "inst >> 6".
+ Store Absolute: stwa, stda (according to Chapter 7, they are only
+ in "inst >> 6".
+ Store Bytes: stby, stdby (according to Chapter 7, they are
+ only in "inst >> 6").
+
+ For (inst >> 26), according to Chapter 7:
+
+ The effective memory reference address is formed by the addition
+ of an immediate displacement to a base value.
+
+ - stb: 0x18, store a byte from a general register.
+
+ - sth: 0x19, store a halfword from a general register.
+
+ - stw: 0x1a, store a word from a general register.
+
+ - stwm: 0x1b, store a word from a general register and perform base
+ register modification (2.0 will still treate it as stw).
+
+ - std: 0x1c, store a doubleword from a general register (2.0 only).
+
+ - stw: 0x1f, store a word from a general register (2.0 only).
+
+ For (inst >> 6) when ((inst >> 26) == 0x03), according to Chapter 7:
+
+ The effective memory reference address is formed by the addition
+ of an index value to a base value specified in the instruction.
+
+ - stb: 0x08, store a byte from a general register (1.1 calls stbs).
+
+ - sth: 0x09, store a halfword from a general register (1.1 calls
+ sths).
+
+ - stw: 0x0a, store a word from a general register (1.1 calls stws).
+
+ - std: 0x0b: store a doubleword from a general register (2.0 only)
+
+ Implement fast byte moves (stores) to unaligned word or doubleword
+ destination.
+
+ - stby: 0x0c, for unaligned word (1.1 calls stbys).
+
+ - stdby: 0x0d for unaligned doubleword (2.0 only).
+
+ Store a word or doubleword using an absolute memory address formed
+ using short or long displacement or indexed
+
+ - stwa: 0x0e, store a word from a general register to an absolute
+ address (1.0 calls stwas).
+
+ - stda: 0x0f, store a doubleword from a general register to an
+ absolute address (2.0 only). */
+
+static int
+inst_saves_gr (unsigned long inst)
+{
+ switch ((inst >> 26) & 0x0f)
+ {
+ case 0x03:
+ switch ((inst >> 6) & 0x0f)
+ {
+ case 0x08:
+ case 0x09:
+ case 0x0a:
+ case 0x0b:
+ case 0x0c:
+ case 0x0d:
+ case 0x0e:
+ case 0x0f:
+ return hppa_extract_5R_store (inst);
+ default:
+ return 0;
+ }
+ case 0x18:
+ case 0x19:
+ case 0x1a:
+ case 0x1b:
+ case 0x1c:
+ /* no 0x1d or 0x1e -- according to parisc 2.0 document */
+ case 0x1f:
+ return hppa_extract_5R_store (inst);
+ default:
+ return 0;
+ }
}
/* Return the register number for a FR which is saved by INST or