Re: [pph] Expect checksums for tests marked with pph asm xdiff (issue4744043)

2011-07-18 Thread Gabriel Charette 
On Sat, Jul 16, 2011 at 12:59 AM, Andreas Schwab sch...@linux-m68k.org wrote:

 gch...@google.com (Gabriel Charette) writes:

  +         set expectedSum [exec tr -d \}  [exec cut -f 4 -d\   
  $xdiff_entry]]

$xdiffentry is set to something like {{pph asm diff 123456}}, so i
need tr and cut to extract 123456

(i tried removing the tr and i get 123456})


  +         set actualSum [exec cut -f 1 -d\   [exec sum  $diff_result]]

and the sum of $diff_result is something like 123456 12, i'm
ignoring the second number for simplicity; hence why i'm using cut
here.

If there are ways to do this directly in TCL, without using exec
calls, I'd be glad to know.

Gabriel

Re: [pph] Expect checksums for tests marked with pph asm xdiff (issue4744043)

2011-07-18 Thread Andreas Schwab 
Gabriel Charette gch...@google.com writes:

 On Sat, Jul 16, 2011 at 12:59 AM, Andreas Schwab sch...@linux-m68k.org 
 wrote:

 gch...@google.com (Gabriel Charette) writes:

  +         set expectedSum [exec tr -d \}  [exec cut -f 4 -d\   
  $xdiff_entry]]

 $xdiffentry is set to something like {{pph asm diff 123456}}, so i
 need tr and cut to extract 123456

tcl can everything tr and cut can do (with the above options anyway).

 If there are ways to do this directly in TCL, without using exec
 calls, I'd be glad to know.

Read split(n) and string(n).

Andreas.

-- 
Andreas Schwab, sch...@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
And now for something completely different.

Re: [pph] Expect checksums for tests marked with pph asm xdiff (issue4744043)

2011-07-18 Thread Gabriel Charette 
On Mon, Jul 18, 2011 at 3:05 PM, Andreas Schwab sch...@linux-m68k.org wrote:
 Gabriel Charette gch...@google.com writes:

 On Sat, Jul 16, 2011 at 12:59 AM, Andreas Schwab sch...@linux-m68k.org 
 wrote:

 gch...@google.com (Gabriel Charette) writes:

  +         set expectedSum [exec tr -d \}  [exec cut -f 4 -d\   
  $xdiff_entry]]

 $xdiffentry is set to something like {{pph asm diff 123456}}, so i
 need tr and cut to extract 123456

 tcl can everything tr and cut can do (with the above options anyway).

 If there are ways to do this directly in TCL, without using exec
 calls, I'd be glad to know.

 Read split(n) and string(n).


Thanks for the suggestion,
suggested changes made as part of issue 4768041.

Gabriel

Re: [pph] Expect checksums for tests marked with pph asm xdiff (issue4744043)

2011-07-16 Thread Andreas Schwab 
gch...@google.com (Gabriel Charette) writes:

 + set expectedSum [exec tr -d \}  [exec cut -f 4 -d\   
 $xdiff_entry]]
 + set actualSum [exec cut -f 1 -d\   [exec sum  $diff_result]]

You don't need cut and tr if you have tcl.

Andreas.

-- 
Andreas Schwab, sch...@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
And now for something completely different.

[pph] Expect checksums for tests marked with pph asm xdiff (issue4744043)

2011-07-15 Thread Gabriel Charette 
This patch adds an expected checksum for the tests expecting an asm diff.

This way if we were expecting an asm diff, still get one, but a different one, 
we know (before this patch we would ignore this, generating an XFAIL as usual, 
as the status of having an asm diff itself hadn't changed).

I had to change from using the TCL grep to the bash grep (through an exec call) 
as I now need the actual output of the grep call, not only the return value (it 
also turns out the return value for TCL grep and bash grep are different; hence 
the change in the if statements on the adiff variable)

Gab

2011-07-15  Gabriel Charette  gch...@google.com

* g++.dg/pph/c1builtin-integral.cc: Add expected diff checksum.
* g++.dg/pph/c1eabi1.cc: Add expected diff checksum.
* g++.dg/pph/p4eabi1.cc: Add expected diff checksum.
* lib/dg-pph.exp (dg-pph-pos): Expect checksums for pph asm xdiff.

diff --git a/gcc/testsuite/g++.dg/pph/c1builtin-integral.cc 
b/gcc/testsuite/g++.dg/pph/c1builtin-integral.cc
index c2fceec..6887b11 100644
--- a/gcc/testsuite/g++.dg/pph/c1builtin-integral.cc
+++ b/gcc/testsuite/g++.dg/pph/c1builtin-integral.cc
@@ -1,2 +1,2 @@
-// pph asm xdiff
+// pph asm xdiff 52758
 #include c0builtin-integral.h
diff --git a/gcc/testsuite/g++.dg/pph/c1eabi1.cc 
b/gcc/testsuite/g++.dg/pph/c1eabi1.cc
index b127f98..3321870 100644
--- a/gcc/testsuite/g++.dg/pph/c1eabi1.cc
+++ b/gcc/testsuite/g++.dg/pph/c1eabi1.cc
@@ -1,5 +1,5 @@
 // { dg-options -w -fpermissive }
-// pph asm xdiff
+// pph asm xdiff 36206
 
 #include c0eabi1.h
 
diff --git a/gcc/testsuite/g++.dg/pph/p4eabi1.cc 
b/gcc/testsuite/g++.dg/pph/p4eabi1.cc
index 4247f49..2f0715f 100644
--- a/gcc/testsuite/g++.dg/pph/p4eabi1.cc
+++ b/gcc/testsuite/g++.dg/pph/p4eabi1.cc
@@ -1,5 +1,5 @@
 // { dg-options -w -fpermissive }
-// pph asm xdiff
+// pph asm xdiff 15662
 
 #include p4eabi1.h
 
diff --git a/gcc/testsuite/lib/dg-pph.exp b/gcc/testsuite/lib/dg-pph.exp
index b706f27..1d7deed 100644
--- a/gcc/testsuite/lib/dg-pph.exp
+++ b/gcc/testsuite/lib/dg-pph.exp
@@ -128,12 +128,11 @@ proc dg-pph-pos { subdir test options mapflag suffix } {
 verbose -log 
 
 # Compare the two assembly files.  They should be identical.
-set adiff [diff $bname.s-pph $bname.s+pph]
+set adiff [catch {exec diff $bname.s-pph $bname.s+pph} diff_result]
 # The sources mark when they expect the comparison to differ.
-set xdiff [llength [grep $test pph asm xdiff]]
+set xdiff_entry [grep $test pph asm xdiff( )*\[0-9\]*]
+set xdiff [llength $xdiff_entry]
 if { $adiff == 0 } {
-   fail $nshort $options comparison failure
-} elseif { $adiff == 1 } {
if { $xdiff } {
xpass $nshort $options (assembly comparison)
} else {
@@ -141,11 +140,20 @@ proc dg-pph-pos { subdir test options mapflag suffix } {
}
file_on_host delete $bname.s-pph
file_on_host delete $bname.s+pph
-} else {
+} elseif { $adiff == 1 } {
+verbose -log Diff obtained:\n$diff_result
if { $xdiff } {
-   xfail $nshort $options (assembly comparison)
+   set expectedSum [exec tr -d \}  [exec cut -f 4 -d\   
$xdiff_entry]]
+   set actualSum [exec cut -f 1 -d\   [exec sum  $diff_result]]
+   if { $expectedSum == $actualSum } {
+   xfail $nshort $options (assembly comparison)
+   } else {
+   fail $nshort $options (assembly comparison, sums differ: 
expected $expectedSum, actual $actualSum)
+   }
} else {
fail $nshort $options (assembly comparison)
}
+} else {
+   fail $nshort $options comparison failure
 }
 }

--
This patch is available for review at http://codereview.appspot.com/4744043

[pph] Expect checksums for tests marked with pph asm xdiff (issue4744043)

2011-07-15 Thread Gabriel Charette 
Reviewed in person by Lawrence.

Shortened XFAIL message from previous patch.

Also, quick side note, in the first description of this patch I said I used 
exec grep, I meant exec diff.

Gab

2011-07-15  Gabriel Charette  gch...@google.com

* g++.dg/pph/c1builtin-integral.cc: Add expected diff checksum.
* g++.dg/pph/c1eabi1.cc: Add expected diff checksum.
* g++.dg/pph/p4eabi1.cc: Add expected diff checksum.
* lib/dg-pph.exp (dg-pph-pos): Expect checksums for pph asm xdiff.

diff --git a/gcc/testsuite/g++.dg/pph/c1builtin-integral.cc 
b/gcc/testsuite/g++.dg/pph/c1builtin-integral.cc
index c2fceec..6887b11 100644
--- a/gcc/testsuite/g++.dg/pph/c1builtin-integral.cc
+++ b/gcc/testsuite/g++.dg/pph/c1builtin-integral.cc
@@ -1,2 +1,2 @@
-// pph asm xdiff
+// pph asm xdiff 52758
 #include c0builtin-integral.h
diff --git a/gcc/testsuite/g++.dg/pph/c1eabi1.cc 
b/gcc/testsuite/g++.dg/pph/c1eabi1.cc
index b127f98..3321870 100644
--- a/gcc/testsuite/g++.dg/pph/c1eabi1.cc
+++ b/gcc/testsuite/g++.dg/pph/c1eabi1.cc
@@ -1,5 +1,5 @@
 // { dg-options -w -fpermissive }
-// pph asm xdiff
+// pph asm xdiff 36206
 
 #include c0eabi1.h
 
diff --git a/gcc/testsuite/g++.dg/pph/p4eabi1.cc 
b/gcc/testsuite/g++.dg/pph/p4eabi1.cc
index 4247f49..2f0715f 100644
--- a/gcc/testsuite/g++.dg/pph/p4eabi1.cc
+++ b/gcc/testsuite/g++.dg/pph/p4eabi1.cc
@@ -1,5 +1,5 @@
 // { dg-options -w -fpermissive }
-// pph asm xdiff
+// pph asm xdiff 15662
 
 #include p4eabi1.h
 
diff --git a/gcc/testsuite/lib/dg-pph.exp b/gcc/testsuite/lib/dg-pph.exp
index b706f27..b285ccf 100644
--- a/gcc/testsuite/lib/dg-pph.exp
+++ b/gcc/testsuite/lib/dg-pph.exp
@@ -128,12 +128,11 @@ proc dg-pph-pos { subdir test options mapflag suffix } {
 verbose -log 
 
 # Compare the two assembly files.  They should be identical.
-set adiff [diff $bname.s-pph $bname.s+pph]
+set adiff [catch {exec diff $bname.s-pph $bname.s+pph} diff_result]
 # The sources mark when they expect the comparison to differ.
-set xdiff [llength [grep $test pph asm xdiff]]
+set xdiff_entry [grep $test pph asm xdiff( )*\[0-9\]*]
+set xdiff [llength $xdiff_entry]
 if { $adiff == 0 } {
-   fail $nshort $options comparison failure
-} elseif { $adiff == 1 } {
if { $xdiff } {
xpass $nshort $options (assembly comparison)
} else {
@@ -141,11 +140,20 @@ proc dg-pph-pos { subdir test options mapflag suffix } {
}
file_on_host delete $bname.s-pph
file_on_host delete $bname.s+pph
-} else {
+} elseif { $adiff == 1 } {
+verbose -log Diff obtained:\n$diff_result
if { $xdiff } {
-   xfail $nshort $options (assembly comparison)
+   set expectedSum [exec tr -d \}  [exec cut -f 4 -d\   
$xdiff_entry]]
+   set actualSum [exec cut -f 1 -d\   [exec sum  $diff_result]]
+   if { $expectedSum == $actualSum } {
+   xfail $nshort $options (assembly comparison)
+   } else {
+   fail $nshort $options (assembly comparison, sums 
$expectedSum=$actualSum)
+   }
} else {
fail $nshort $options (assembly comparison)
}
+} else {
+   fail $nshort $options comparison failure
 }
 }

--
This patch is available for review at http://codereview.appspot.com/4744043

Re: [pph] Expect checksums for tests marked with pph asm xdiff (issue4744043)

2011-07-15 Thread dnovillo 

OK with the change below.


Diego.


http://codereview.appspot.com/4744043/diff/3001/gcc/testsuite/lib/dg-pph.exp
File gcc/testsuite/lib/dg-pph.exp (right):

http://codereview.appspot.com/4744043/diff/3001/gcc/testsuite/lib/dg-pph.exp#newcode131
gcc/testsuite/lib/dg-pph.exp:131: set adiff [catch {exec diff
$bname.s-pph $bname.s+pph} diff_result]
131 set adiff [catch {exec diff $bname.s-pph $bname.s+pph}
diff_result]

You don't need this, if instead of summing the diff you sum
$bname.s+pph.  The logic would then be: if there is a difference between
$bname.s-pph and $bname.s+pph, we checksum $bname.s+pph.  If the new
checksum is different than the stored one, it means that $bname.s+pph is
different from $bname.s-pph in a different way.

This has the benefit of:

- Being slightly faster.
- Simplifies the generation of checksums.  We no longer need to checksum
the difference between the two .s files, we just need to checksum the
.s+pph file

http://codereview.appspot.com/4744043/

Re: [pph] Expect checksums for tests marked with pph asm xdiff (issue4744043)

2011-07-15 Thread Lawrence Crowl 
On 7/15/11, Gabriel Charette gch...@google.com wrote:
 This patch adds an expected checksum for the tests expecting an asm diff.

 This way if we were expecting an asm diff, still get one, but a different
 one, we know (before this patch we would ignore this, generating an XFAIL as
 usual, as the status of having an asm diff itself hadn't changed).

 I had to change from using the TCL grep to the bash grep (through an exec
 call) as I now need the actual output of the grep call, not only the return
 value (it also turns out the return value for TCL grep and bash grep are
 different; hence the change in the if statements on the adiff variable)

 Gab

 2011-07-15  Gabriel Charette  gch...@google.com

   * g++.dg/pph/c1builtin-integral.cc: Add expected diff checksum.
   * g++.dg/pph/c1eabi1.cc: Add expected diff checksum.
   * g++.dg/pph/p4eabi1.cc: Add expected diff checksum.
   * lib/dg-pph.exp (dg-pph-pos): Expect checksums for pph asm xdiff.

 diff --git a/gcc/testsuite/g++.dg/pph/c1builtin-integral.cc
 b/gcc/testsuite/g++.dg/pph/c1builtin-integral.cc
 index c2fceec..6887b11 100644
 --- a/gcc/testsuite/g++.dg/pph/c1builtin-integral.cc
 +++ b/gcc/testsuite/g++.dg/pph/c1builtin-integral.cc
 @@ -1,2 +1,2 @@
 -// pph asm xdiff
 +// pph asm xdiff 52758
  #include c0builtin-integral.h
 diff --git a/gcc/testsuite/g++.dg/pph/c1eabi1.cc
 b/gcc/testsuite/g++.dg/pph/c1eabi1.cc
 index b127f98..3321870 100644
 --- a/gcc/testsuite/g++.dg/pph/c1eabi1.cc
 +++ b/gcc/testsuite/g++.dg/pph/c1eabi1.cc
 @@ -1,5 +1,5 @@
  // { dg-options -w -fpermissive }
 -// pph asm xdiff
 +// pph asm xdiff 36206

  #include c0eabi1.h

 diff --git a/gcc/testsuite/g++.dg/pph/p4eabi1.cc
 b/gcc/testsuite/g++.dg/pph/p4eabi1.cc
 index 4247f49..2f0715f 100644
 --- a/gcc/testsuite/g++.dg/pph/p4eabi1.cc
 +++ b/gcc/testsuite/g++.dg/pph/p4eabi1.cc
 @@ -1,5 +1,5 @@
  // { dg-options -w -fpermissive }
 -// pph asm xdiff
 +// pph asm xdiff 15662

  #include p4eabi1.h

 diff --git a/gcc/testsuite/lib/dg-pph.exp b/gcc/testsuite/lib/dg-pph.exp
 index b706f27..1d7deed 100644
 --- a/gcc/testsuite/lib/dg-pph.exp
 +++ b/gcc/testsuite/lib/dg-pph.exp
 @@ -128,12 +128,11 @@ proc dg-pph-pos { subdir test options mapflag suffix }
 {
  verbose -log 

  # Compare the two assembly files.  They should be identical.
 -set adiff [diff $bname.s-pph $bname.s+pph]
 +set adiff [catch {exec diff $bname.s-pph $bname.s+pph} diff_result]
  # The sources mark when they expect the comparison to differ.
 -set xdiff [llength [grep $test pph asm xdiff]]
 +set xdiff_entry [grep $test pph asm xdiff( )*\[0-9\]*]
 +set xdiff [llength $xdiff_entry]
  if { $adiff == 0 } {
 - fail $nshort $options comparison failure
 -} elseif { $adiff == 1 } {
   if { $xdiff } {
   xpass $nshort $options (assembly comparison)
   } else {
 @@ -141,11 +140,20 @@ proc dg-pph-pos { subdir test options mapflag suffix }
 {
   }
   file_on_host delete $bname.s-pph
   file_on_host delete $bname.s+pph
 -} else {
 +} elseif { $adiff == 1 } {
 +verbose -log Diff obtained:\n$diff_result
   if { $xdiff } {
 - xfail $nshort $options (assembly comparison)
 + set expectedSum [exec tr -d \}  [exec cut -f 4 -d\  
 $xdiff_entry]]
 + set actualSum [exec cut -f 1 -d\   [exec sum  $diff_result]]
 + if { $expectedSum == $actualSum } {
 + xfail $nshort $options (assembly comparison)
 + } else {
 + fail $nshort $options (assembly comparison, sums differ: 
 expected
 $expectedSum, actual $actualSum)
 + }
   } else {
   fail $nshort $options (assembly comparison)
   }
 +} else {
 + fail $nshort $options comparison failure
  }
  }

 --
 This patch is available for review at http://codereview.appspot.com/4744043


Needs shortening of message.  Otherwise, LGTM.

-- 
Lawrence Crowl

Re: [pph] Expect checksums for tests marked with pph asm xdiff (issue4744043)

2011-07-15 Thread Lawrence Crowl 
On 7/15/11, Lawrence Crowl cr...@google.com wrote:
 On 7/15/11, Gabriel Charette gch...@google.com wrote:
 This patch adds an expected checksum for the tests expecting an asm diff.

 This way if we were expecting an asm diff, still get one, but a different
 one, we know (before this patch we would ignore this, generating an XFAIL
 as
 usual, as the status of having an asm diff itself hadn't changed).

 I had to change from using the TCL grep to the bash grep (through an exec
 call) as I now need the actual output of the grep call, not only the
 return
 value (it also turns out the return value for TCL grep and bash grep are
 different; hence the change in the if statements on the adiff variable)

 Gab

 2011-07-15  Gabriel Charette  gch...@google.com

  * g++.dg/pph/c1builtin-integral.cc: Add expected diff checksum.
  * g++.dg/pph/c1eabi1.cc: Add expected diff checksum.
  * g++.dg/pph/p4eabi1.cc: Add expected diff checksum.
  * lib/dg-pph.exp (dg-pph-pos): Expect checksums for pph asm xdiff.

 diff --git a/gcc/testsuite/g++.dg/pph/c1builtin-integral.cc
 b/gcc/testsuite/g++.dg/pph/c1builtin-integral.cc
 index c2fceec..6887b11 100644
 --- a/gcc/testsuite/g++.dg/pph/c1builtin-integral.cc
 +++ b/gcc/testsuite/g++.dg/pph/c1builtin-integral.cc
 @@ -1,2 +1,2 @@
 -// pph asm xdiff
 +// pph asm xdiff 52758
  #include c0builtin-integral.h
 diff --git a/gcc/testsuite/g++.dg/pph/c1eabi1.cc
 b/gcc/testsuite/g++.dg/pph/c1eabi1.cc
 index b127f98..3321870 100644
 --- a/gcc/testsuite/g++.dg/pph/c1eabi1.cc
 +++ b/gcc/testsuite/g++.dg/pph/c1eabi1.cc
 @@ -1,5 +1,5 @@
  // { dg-options -w -fpermissive }
 -// pph asm xdiff
 +// pph asm xdiff 36206

  #include c0eabi1.h

 diff --git a/gcc/testsuite/g++.dg/pph/p4eabi1.cc
 b/gcc/testsuite/g++.dg/pph/p4eabi1.cc
 index 4247f49..2f0715f 100644
 --- a/gcc/testsuite/g++.dg/pph/p4eabi1.cc
 +++ b/gcc/testsuite/g++.dg/pph/p4eabi1.cc
 @@ -1,5 +1,5 @@
  // { dg-options -w -fpermissive }
 -// pph asm xdiff
 +// pph asm xdiff 15662

  #include p4eabi1.h

 diff --git a/gcc/testsuite/lib/dg-pph.exp b/gcc/testsuite/lib/dg-pph.exp
 index b706f27..1d7deed 100644
 --- a/gcc/testsuite/lib/dg-pph.exp
 +++ b/gcc/testsuite/lib/dg-pph.exp
 @@ -128,12 +128,11 @@ proc dg-pph-pos { subdir test options mapflag suffix
 }
 {
  verbose -log 

  # Compare the two assembly files.  They should be identical.
 -set adiff [diff $bname.s-pph $bname.s+pph]
 +set adiff [catch {exec diff $bname.s-pph $bname.s+pph}
 diff_result]
  # The sources mark when they expect the comparison to differ.
 -set xdiff [llength [grep $test pph asm xdiff]]
 +set xdiff_entry [grep $test pph asm xdiff( )*\[0-9\]*]
 +set xdiff [llength $xdiff_entry]
  if { $adiff == 0 } {
 -fail $nshort $options comparison failure
 -} elseif { $adiff == 1 } {
  if { $xdiff } {
  xpass $nshort $options (assembly comparison)
  } else {
 @@ -141,11 +140,20 @@ proc dg-pph-pos { subdir test options mapflag suffix
 }
 {
  }
  file_on_host delete $bname.s-pph
  file_on_host delete $bname.s+pph
 -} else {
 +} elseif { $adiff == 1 } {
 +verbose -log Diff obtained:\n$diff_result
  if { $xdiff } {
 -xfail $nshort $options (assembly comparison)
 +set expectedSum [exec tr -d \}  [exec cut -f 4 -d\  
 $xdiff_entry]]
 +set actualSum [exec cut -f 1 -d\   [exec sum  $diff_result]]
 +if { $expectedSum == $actualSum } {
 +xfail $nshort $options (assembly comparison)
 +} else {
 +fail $nshort $options (assembly comparison, sums differ:
 expected
 $expectedSum, actual $actualSum)
 +}
  } else {
  fail $nshort $options (assembly comparison)
  }
 +} else {
 +fail $nshort $options comparison failure
  }
  }

 --
 This patch is available for review at
 http://codereview.appspot.com/4744043

 Needs shortening of message.  Otherwise, LGTM.

We have crossed the streams.  LGTM.

-- 
Lawrence Crowl

Re: [pph] Expect checksums for tests marked with pph asm xdiff (issue4744043)

2011-07-15 Thread gchare 

See below.


http://codereview.appspot.com/4744043/diff/3001/gcc/testsuite/lib/dg-pph.exp
File gcc/testsuite/lib/dg-pph.exp (right):

http://codereview.appspot.com/4744043/diff/3001/gcc/testsuite/lib/dg-pph.exp#newcode131
gcc/testsuite/lib/dg-pph.exp:131: set adiff [catch {exec diff
$bname.s-pph $bname.s+pph} diff_result]
I actually prefer checksum on the diff itself:
It's less affected by merges (in particular the merge timestamp doesn't
show up in the diff, so unless a merge from trunk changes the actual
assembly, the diff is the same as before)

Also, the generation of checksums is not harder this way, I made it so
the tests' output tells you what the expected/actual sums are when they
differ, so no need to generate them by hand.

http://codereview.appspot.com/4744043/

Re: [pph] Expect checksums for tests marked with pph asm xdiff (issue4744043)

2011-07-15 Thread Diego Novillo 
On Fri, Jul 15, 2011 at 19:21,  gch...@google.com wrote:

 It's less affected by merges (in particular the merge timestamp doesn't
 show up in the diff, so unless a merge from trunk changes the actual
 assembly, the diff is the same as before)

Ah, good point.

 Also, the generation of checksums is not harder this way, I made it so
 the tests' output tells you what the expected/actual sums are when they
 differ, so no need to generate them by hand.

OK, in that case go ahead.


Diego.