[v3] gdb/hppa-tdep.c: Fix logical working flow issues and check additional store instructions

Message ID 548C713F.1080500@gmail.com
State New, archived
Headers

Commit Message

Chen Gang Dec. 13, 2014, 5:02 p.m. UTC
  Original working flow has several issues:

 - typo issue: "(inst >> 26) == 0x1f && ..." for checking 'stw(m)'.

 - compiler warning: "(inst >> 6) & 0xf) == 0x9" for checking 'sth'.

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-14  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 | 111 +++++++++++++++++++++++++++++++++++++++++++-------------
 1 file changed, 85 insertions(+), 26 deletions(-)
  

Comments

Chen Gang Dec. 14, 2014, 12:16 p.m. UTC | #1
Oh, sorry, the comments still need completion, it is 2 areas below.

If necessary to send patch v4 fot them, please let me know, thanks.


On 12/14/14 01:02, Chen Gang wrote:
> 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-14  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 | 111 +++++++++++++++++++++++++++++++++++++++++++-------------
>  1 file changed, 85 insertions(+), 26 deletions(-)
> 
> diff --git a/gdb/hppa-tdep.c b/gdb/hppa-tdep.c
> index 627f31a..5983193 100644
> --- a/gdb/hppa-tdep.c
> +++ b/gdb/hppa-tdep.c
> @@ -1376,37 +1376,96 @@ 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 it 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);
> +   For (inst >> 26):
>  
> -  return 0;
> +     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 treat 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):
> +
> +     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:
> +	      break;
> +	    default:
> +	      return 0;
> +	  }
> +	/* Fall through */
> +      case 0x18:
> +      case 0x19:
> +      case 0x1a:
> +      case 0x1b:
> +      case 0x1c:
> +      /* no 0x1d, according to parisc 2.0 document */

         /* 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
> 


Thanks.
  
Joel Brobecker Dec. 14, 2014, 1:22 p.m. UTC | #2
> Oh, sorry, the comments still need completion, it is 2 areas below.
> 
> If necessary to send patch v4 fot them, please let me know, thanks.

You could have just attached it here :). Yes please, send us what
you'd like us to review.
  

Patch

diff --git a/gdb/hppa-tdep.c b/gdb/hppa-tdep.c
index 627f31a..5983193 100644
--- a/gdb/hppa-tdep.c
+++ b/gdb/hppa-tdep.c
@@ -1376,37 +1376,96 @@  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 it 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);
+   For (inst >> 26):
 
-  return 0;
+     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 treat 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):
+
+     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:
+	      break;
+	    default:
+	      return 0;
+	  }
+	/* Fall through */
+      case 0x18:
+      case 0x19:
+      case 0x1a:
+      case 0x1b:
+      case 0x1c:
+      /* no 0x1d, 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