RFC: Some more store.exp failures - tweak the test

RFC: Some more store.exp failures - tweak the test

[Daniel Jacobowitz]

This patch fixes the store.exp failures for ARM.  Two changes:
  - Change char to signed char, because some patterns match -1.  If anyone
    prefers changing the patterns to match 255 would work too.
  - Change "return l" to "return l + r".  "up; print r" doesn't work if
    "r" is not live across the function call; even without optimization
    GCC will re-use the register.  Then we lose.

I believe these changes don't impact the point of the test.  If nobody
disagrees with me, I'd like to commit this.

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

2003-07-01  Daniel Jacobowitz  <drow@mvista.com>

	* gdb.base/store.c (wack_char): Make l and r signed characters.
	Return l + r to keep r live across the call.
	(wack_short, wack_int, wack_long, wack_longest, wack_float)
	(wack_double, wack_doublest): Return l + r to keep r live across
	the call.
	* gdb.base/store.exp: Accomodate store.c changes.

Index: gdb/testsuite/gdb.base/store.c
===================================================================
--- gdb.orig/testsuite/gdb.base/store.c	2003-06-14 18:10:47.000000000 -0400
+++ gdb/testsuite/gdb.base/store.c	2003-07-01 17:05:58.000000000 -0400
@@ -64,9 +64,9 @@ add_doublest (register doublest u, regis
 char
 wack_char (register char u, register char v)
 {
-  register char l = u, r = v;
+  register signed char l = u, r = v;
   l = add_char (l, r);
-  return l;
+  return l + r;
 }
 
 short
@@ -74,7 +74,7 @@ wack_short (register short u, register s
 {
   register short l = u, r = v;
   l = add_short (l, r);
-  return l;
+  return l + r;
 }
 
 int
@@ -82,7 +82,7 @@ wack_int (register int u, register int v
 {
   register int l = u, r = v;
   l = add_int (l, r);
-  return l;
+  return l + r;
 }
 
 long
@@ -90,7 +90,7 @@ wack_long (register long u, register lon
 {
   register long l = u, r = v;
   l = add_long (l, r);
-  return l;
+  return l + r;
 }
 
 long
@@ -98,7 +98,7 @@ wack_longest (register longest u, regist
 {
   register longest l = u, r = v;
   l = add_longest (l, r);
-  return l;
+  return l + r;
 }
 
 float
@@ -106,7 +106,7 @@ wack_float (register float u, register f
 {
   register float l = u, r = v;
   l = add_float (l, r);
-  return l;
+  return l + r;
 }
 
 double
@@ -114,7 +114,7 @@ wack_double (register double u, register
 {
   register double l = u, r = v;
   l = add_double (l, r);
-  return l;
+  return l + r;
 }
 
 doublest
@@ -122,7 +122,7 @@ wack_doublest (register doublest u, regi
 {
   register doublest l = u, r = v;
   l = add_doublest (l, r);
-  return l;
+  return l + r;
 }
 
 /* */
Index: gdb/testsuite/gdb.base/store.exp
===================================================================
--- gdb.orig/testsuite/gdb.base/store.exp	2003-06-14 18:10:47.000000000 -0400
+++ gdb/testsuite/gdb.base/store.exp	2003-07-01 17:06:52.000000000 -0400
@@ -57,13 +57,13 @@ if ![runto_main] then {
 proc check_set { t l r new add } {
     global gdb_prompt
     gdb_test "tbreak wack_${t}"
-    gdb_test "continue" "register ${t} l = u, r = v;" "continue to wack_${t}"
+    gdb_test "continue" "register (signed )?${t} l = u, r = v;" "continue to wack_${t}"
     gdb_test "next" "l = add_${t} .l, r.;" "next ${t}"
     gdb_test "print l" " = ${l}" "print old l - ${t}"
     gdb_test "print r" " = ${r}" "print old r - ${t}"
     gdb_test "set variable l = 4"
     gdb_test "print l" " = ${new}" "print new l - ${t}"
-    gdb_test "next" "return l;"
+    gdb_test "next" "return l \\+ r;"
     gdb_test "print l" " = ${add}" "print add  - ${t}"
 }
 

Re: RFC: Some more store.exp failures - tweak the test

[Andrew Cagney]

> This patch fixes the store.exp failures for ARM.  Two changes:
>   - Change char to signed char, because some patterns match -1.  If anyone
>     prefers changing the patterns to match 255 would work too.

Yes, but use something like `typedef signed char charest' so that the 
function's pattern doesn't need tweaking.

>   - Change "return l" to "return l + r".  "up; print r" doesn't work if
>     "r" is not live across the function call; even without optimization
>     GCC will re-use the register.  Then we lose.

As an aside, isn't that a GCC bug?  If no optimization, shouldn't "r" 
have a single permenant location?  The user's going to expect it to 
work, no matter how dumb their code.

> I believe these changes don't impact the point of the test.  If nobody
> disagrees with me, I'd like to commit this.

Andrew

Re: RFC: Some more store.exp failures - tweak the test

[Daniel Jacobowitz]

On Tue, Jul 01, 2003 at 06:22:29PM -0400, Andrew Cagney wrote:
> >This patch fixes the store.exp failures for ARM.  Two changes:
> >  - Change char to signed char, because some patterns match -1.  If anyone
> >    prefers changing the patterns to match 255 would work too.
> 
> Yes, but use something like `typedef signed char charest' so that the 
> function's pattern doesn't need tweaking.

OK.

> >  - Change "return l" to "return l + r".  "up; print r" doesn't work if
> >    "r" is not live across the function call; even without optimization
> >    GCC will re-use the register.  Then we lose.
> 
> As an aside, isn't that a GCC bug?  If no optimization, shouldn't "r" 
> have a single permenant location?  The user's going to expect it to 
> work, no matter how dumb their code.

Hmm, maybe.  I'll ask a couple of GCC folks what they think, but I
doubt this will ever change - -O0 code is horrible enough already.

> >I believe these changes don't impact the point of the test.  If nobody
> >disagrees with me, I'd like to commit this.

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

Re: RFC: Some more store.exp failures - tweak the test

[Daniel Jacobowitz]

On Tue, Jul 01, 2003 at 06:22:29PM -0400, Andrew Cagney wrote:
> >This patch fixes the store.exp failures for ARM.  Two changes:
> >  - Change char to signed char, because some patterns match -1.  If anyone
> >    prefers changing the patterns to match 255 would work too.
> 
> Yes, but use something like `typedef signed char charest' so that the 
> function's pattern doesn't need tweaking.

Good idea.  Checked in with that change.

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

2003-07-03  Daniel Jacobowitz  <drow@mvista.com>

	* gdb.base/store.c (charest): New typedef.
	(add_char): Rename to add_charest, update.
	(wack_char): Rename to wack_charest, update types.  Return l + r
	to keep r live across the call.
	(wack_short, wack_int, wack_long, wack_longest, wack_float)
	(wack_double, wack_doublest): Return l + r to keep r live across
	the call.
	* gdb.base/store.exp: Accomodate store.c changes.

Index: gdb/testsuite/gdb.base/store.c
===================================================================
--- gdb.orig/testsuite/gdb.base/store.c	2003-07-01 17:56:38.000000000 -0400
+++ gdb/testsuite/gdb.base/store.c	2003-07-03 09:46:53.000000000 -0400
@@ -7,8 +7,10 @@
    function calls within main even when no optimization flags were
    passed.  */
 
-char
-add_char (register char u, register char v)
+typedef signed char charest;
+
+charest
+add_charest (register charest u, register charest v)
 {
   return u + v;
 }
@@ -61,12 +63,12 @@ add_doublest (register doublest u, regis
 
 /* */
 
-char
-wack_char (register char u, register char v)
+charest
+wack_charest (register charest u, register charest v)
 {
-  register char l = u, r = v;
-  l = add_char (l, r);
-  return l;
+  register charest l = u, r = v;
+  l = add_charest (l, r);
+  return l + r;
 }
 
 short
@@ -74,7 +76,7 @@ wack_short (register short u, register s
 {
   register short l = u, r = v;
   l = add_short (l, r);
-  return l;
+  return l + r;
 }
 
 int
@@ -82,7 +84,7 @@ wack_int (register int u, register int v
 {
   register int l = u, r = v;
   l = add_int (l, r);
-  return l;
+  return l + r;
 }
 
 long
@@ -90,7 +92,7 @@ wack_long (register long u, register lon
 {
   register long l = u, r = v;
   l = add_long (l, r);
-  return l;
+  return l + r;
 }
 
 long
@@ -98,7 +100,7 @@ wack_longest (register longest u, regist
 {
   register longest l = u, r = v;
   l = add_longest (l, r);
-  return l;
+  return l + r;
 }
 
 float
@@ -106,7 +108,7 @@ wack_float (register float u, register f
 {
   register float l = u, r = v;
   l = add_float (l, r);
-  return l;
+  return l + r;
 }
 
 double
@@ -114,7 +116,7 @@ wack_double (register double u, register
 {
   register double l = u, r = v;
   l = add_double (l, r);
-  return l;
+  return l + r;
 }
 
 doublest
@@ -122,7 +124,7 @@ wack_doublest (register doublest u, regi
 {
   register doublest l = u, r = v;
   l = add_doublest (l, r);
-  return l;
+  return l + r;
 }
 
 /* */
@@ -253,7 +255,7 @@ int
 main ()
 {
   /* These calls are for current frame test.  */
-  wack_char (-1, -2);
+  wack_charest (-1, -2);
   wack_short (-1, -2);
   wack_int (-1, -2);
   wack_long (-1, -2);
@@ -263,7 +265,7 @@ main ()
   wack_doublest (-1, -2);
 
   /* These calls are for up frame.  */
-  wack_char (-1, -2);
+  wack_charest (-1, -2);
   wack_short (-1, -2);
   wack_int (-1, -2);
   wack_long (-1, -2);
Index: gdb/testsuite/gdb.base/store.exp
===================================================================
--- gdb.orig/testsuite/gdb.base/store.exp	2003-07-01 17:56:38.000000000 -0400
+++ gdb/testsuite/gdb.base/store.exp	2003-07-03 09:48:05.000000000 -0400
@@ -63,11 +63,11 @@ proc check_set { t l r new add } {
     gdb_test "print r" " = ${r}" "print old r - ${t}"
     gdb_test "set variable l = 4"
     gdb_test "print l" " = ${new}" "print new l - ${t}"
-    gdb_test "next" "return l;"
+    gdb_test "next" "return l \\+ r;"
     gdb_test "print l" " = ${add}" "print add  - ${t}"
 }
 
-check_set "char" "-1 .*" "-2 .*" "4 ..004." "2 ..002."
+check_set "charest" "-1 .*" "-2 .*" "4 ..004." "2 ..002."
 check_set "short" "-1" "-2" "4" "2"
 check_set "int" "-1" "-2" "4" "2"
 check_set "long" "-1" "-2" "4" "2"
@@ -89,7 +89,7 @@ proc up_set { t l r new } {
     gdb_test "print l" " = ${new}" "up print new l - ${t}"
 }
 
-up_set "char" "-1 .*" "-2 .*" "4 ..004."
+up_set "charest" "-1 .*" "-2 .*" "4 ..004."
 up_set "short" "-1" "-2" "4"
 up_set "int" "-1" "-2" "4"
 up_set "long" "-1" "-2" "4"

Re: RFC: Some more store.exp failures - tweak the test

[Michael Elizabeth Chastain]

Proofread, not tested.  These changes are okay with me, but I don't want
to go very far in the direction of changing test programs like this.
The 'l + r' thing is innocuous, but the 'signed char' thing bugs me a
little.

drow> I believe these changes don't impact the point of the test.  If nobody
drow> disagrees with me, I'd like to commit this.

Not disagreeing with this case, just advising caution about changes like
this.

Michael C

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

2003-07-01  Daniel Jacobowitz  <drow@mvista.com>

	* gdb.base/store.c (wack_char): Make l and r signed characters.
	Return l + r to keep r live across the call.
	(wack_short, wack_int, wack_long, wack_longest, wack_float)
	(wack_double, wack_doublest): Return l + r to keep r live across
	the call.
	* gdb.base/store.exp: Accomodate store.c changes.