Patchwork [go] Add -mieee to various go.test tests

login
register
mail settings
Submitter Uros Bizjak
Date Jan. 19, 2012, 9:52 p.m.
Message ID <CAFULd4ZV5W+2w06SiHHPk_qMefVzTBTsaYztdy-aby8Gex4nig@mail.gmail.com>
Download mbox | patch
Permalink /patch/136913/
State New
Headers show

Comments

Uros Bizjak - Jan. 19, 2012, 9:52 p.m.
Hello!

Attached patch adds -mieee to tests that need full IEEE compliance to
pass. While working on patch, I have noticed that go-test.exp doesn't
pass DEFAULT_GOCFLAGS to go_target_compile procedure in expected
format (so, these simply get ignored). With this issue fixed, we can
add -mieee to DEFAULT_GOCFLAGS. Tests, compiled through torture
procedure, expects their special flags in a separate driver file.

Attached patch fixes all "floating point errors" on
alphaev68-pc-linux-gnu through these two methods.

2012-01-19  Uros Bizjak  <ubizjak@gmail.com>

	* go.test/go-test.exp (go-gc-tests): Add -mieee to DEFAULT_GOCFLAGS
	for alpha*-*-* and sh*-*-* to enable full IEEE compliance mode.
	Pass correctly formatted options to go_target_compile.
	* go.test/test/fixedbugs/bug321.x: New file.
	* go.test/test/zerodivide.x: Ditto.
	* go.test/test/floatcmp.x: Ditto.

Patch was tested on alphaev68-pc-linux-gnu and x86_64-pc-linux-gnu.

OK for mainline SVN?

Uros.
Ian Taylor - Jan. 19, 2012, 11:39 p.m.
Uros Bizjak <ubizjak@gmail.com> writes:

> Attached patch adds -mieee to tests that need full IEEE compliance to
> pass. While working on patch, I have noticed that go-test.exp doesn't
> pass DEFAULT_GOCFLAGS to go_target_compile procedure in expected
> format (so, these simply get ignored). With this issue fixed, we can
> add -mieee to DEFAULT_GOCFLAGS. Tests, compiled through torture
> procedure, expects their special flags in a separate driver file.
>
> Attached patch fixes all "floating point errors" on
> alphaev68-pc-linux-gnu through these two methods.
>
> 2012-01-19  Uros Bizjak  <ubizjak@gmail.com>
>
> 	* go.test/go-test.exp (go-gc-tests): Add -mieee to DEFAULT_GOCFLAGS
> 	for alpha*-*-* and sh*-*-* to enable full IEEE compliance mode.
> 	Pass correctly formatted options to go_target_compile.
> 	* go.test/test/fixedbugs/bug321.x: New file.
> 	* go.test/test/zerodivide.x: Ditto.
> 	* go.test/test/floatcmp.x: Ditto.
>
> Patch was tested on alphaev68-pc-linux-gnu and x86_64-pc-linux-gnu.
>
> OK for mainline SVN?

I did implement support for .x files in go-test.exp for some reason, but
actually I should not have.  I want the contents of the go.test/test
directory to be an exact copy of the master Go testsuite except for the
README.gcc file, which it is.  So I would like to see this fixed in some
other way.  Sorry.

And I just have to repeat that this patch is an ugly ugly hack, since
-mieee should be the default.  Perhaps we should investigate having
gcc/go/gospec.c or gcc/go/lang-specs.h somehow add -mieee for those
targets which require it.

Ian
Mike Stump - Jan. 20, 2012, 2:16 a.m.
On Jan 19, 2012, at 3:39 PM, Ian Lance Taylor wrote:
> And I just have to repeat that this patch is an ugly ugly hack, since
> -mieee should be the default.  Perhaps we should investigate having
> gcc/go/gospec.c or gcc/go/lang-specs.h somehow add -mieee for those
> targets which require it.

I agree.  One way forward may be to make -mieee a MI flag then you can portably introduce it unconditionally into the specs (ick) or into the option processing code to default it on for go (and java).  Don't know about Ada or Fortran.
Uros Bizjak - Jan. 20, 2012, 9:59 a.m.
On Fri, Jan 20, 2012 at 12:39 AM, Ian Lance Taylor <iant@google.com> wrote:

>> Attached patch adds -mieee to tests that need full IEEE compliance to
>> pass. While working on patch, I have noticed that go-test.exp doesn't
>> pass DEFAULT_GOCFLAGS to go_target_compile procedure in expected
>> format (so, these simply get ignored). With this issue fixed, we can
>> add -mieee to DEFAULT_GOCFLAGS. Tests, compiled through torture
>> procedure, expects their special flags in a separate driver file.
>>
>> Attached patch fixes all "floating point errors" on
>> alphaev68-pc-linux-gnu through these two methods.
>>
>> 2012-01-19  Uros Bizjak  <ubizjak@gmail.com>
>>
>>       * go.test/go-test.exp (go-gc-tests): Add -mieee to DEFAULT_GOCFLAGS
>>       for alpha*-*-* and sh*-*-* to enable full IEEE compliance mode.
>>       Pass correctly formatted options to go_target_compile.
>>       * go.test/test/fixedbugs/bug321.x: New file.
>>       * go.test/test/zerodivide.x: Ditto.
>>       * go.test/test/floatcmp.x: Ditto.
>>
>> Patch was tested on alphaev68-pc-linux-gnu and x86_64-pc-linux-gnu.
>>
>> OK for mainline SVN?
>
> I did implement support for .x files in go-test.exp for some reason, but
> actually I should not have.  I want the contents of the go.test/test
> directory to be an exact copy of the master Go testsuite except for the
> README.gcc file, which it is.  So I would like to see this fixed in some
> other way.  Sorry.
>
> And I just have to repeat that this patch is an ugly ugly hack, since
> -mieee should be the default.  Perhaps we should investigate having
> gcc/go/gospec.c or gcc/go/lang-specs.h somehow add -mieee for those
> targets which require it.

OK, I agree. But please note that the patch also includes a fix to
pass correctly formatted options to go_target_compile. I guess this
part should be installed anyway, otherwise go_target_compile simply
ignores passed extra options.

Uros.
Ian Taylor - Jan. 20, 2012, 2:02 p.m.
Uros Bizjak <ubizjak@gmail.com> writes:

> OK, I agree. But please note that the patch also includes a fix to
> pass correctly formatted options to go_target_compile. I guess this
> part should be installed anyway, otherwise go_target_compile simply
> ignores passed extra options.

Good point, that part of the patch is OK for mainline.

Thanks.

Ian
Tom Tromey - Jan. 20, 2012, 2:26 p.m.
>>>>> "Ian" == Ian Lance Taylor <iant@google.com> writes:

Ian> And I just have to repeat that this patch is an ugly ugly hack, since
Ian> -mieee should be the default.  Perhaps we should investigate having
Ian> gcc/go/gospec.c or gcc/go/lang-specs.h somehow add -mieee for those
Ian> targets which require it.

For gcj, we have libjava provide a specs file that the driver reads.
We stick -mieee in there for Alpha; see libjava/configure.host.

We needed these specs for other reasons -- different ways to configure
libjava require different options to be passed to jc1, and we wanted to
keep the configury in one spot.  So, this was practical in our
situation, I don't know whether it would be more of a pain for you.

Tom
Ian Taylor - Jan. 20, 2012, 3 p.m.
Tom Tromey <tromey@redhat.com> writes:

>>>>>> "Ian" == Ian Lance Taylor <iant@google.com> writes:
>
> Ian> And I just have to repeat that this patch is an ugly ugly hack, since
> Ian> -mieee should be the default.  Perhaps we should investigate having
> Ian> gcc/go/gospec.c or gcc/go/lang-specs.h somehow add -mieee for those
> Ian> targets which require it.
>
> For gcj, we have libjava provide a specs file that the driver reads.
> We stick -mieee in there for Alpha; see libjava/configure.host.

Thanks for the pointer.  That's elegantly complex.

Ian

Patch

Index: test/zerodivide.x
===================================================================
--- test/zerodivide.x	(revision 0)
+++ test/zerodivide.x	(revision 0)
@@ -0,0 +1,6 @@ 
+if { [istarget "alpha*-*-*"] || [istarget "sh*-*-*"] } {
+        # alpha and SH require -mieee for this test.
+        set additional_flags "-mieee"
+}
+
+return 0
Index: test/floatcmp.x
===================================================================
--- test/floatcmp.x	(revision 0)
+++ test/floatcmp.x	(revision 0)
@@ -0,0 +1,6 @@ 
+if { [istarget "alpha*-*-*"] || [istarget "sh*-*-*"] } {
+        # alpha and SH require -mieee for this test.
+        set additional_flags "-mieee"
+}
+
+return 0
Index: test/fixedbugs/bug321.x
===================================================================
--- test/fixedbugs/bug321.x	(revision 0)
+++ test/fixedbugs/bug321.x	(revision 0)
@@ -0,0 +1,6 @@ 
+if { [istarget "alpha*-*-*"] || [istarget "sh*-*-*"] } {
+        # alpha and SH require -mieee for this test.
+        set additional_flags "-mieee"
+}
+
+return 0
Index: go-test.exp
===================================================================
--- go-test.exp	(revision 183305)
+++ go-test.exp	(working copy)
@@ -208,6 +208,15 @@ 
 	set DEFAULT_GOCFLAGS " -pedantic-errors"
     }
 
+    # Enable full IEEE compliance mode.
+    if { [istarget alpha*-*-*]
+         || [istarget sh*-*-*] } then {
+	lappend DEFAULT_GOCFLAGS "-mieee"
+    }
+
+    set options ""
+    lappend options "additional_flags=$DEFAULT_GOCFLAGS"
+
     # Set GOARCH for tests that need it.
     go-set-goarch
 
@@ -585,7 +594,7 @@ 
 	    set dg-do-what-default "link"
 	    set output_file "./[file rootname [file tail $test]].exe"
 	    set comp_output [go_target_compile "$ofile1 $ofile2" \
-				 $output_file "executable" "$DEFAULT_GOCFLAGS"]
+				 $output_file "executable" "$options"]
 	    set comp_output [go-dg-prune $target_triplet $comp_output]
 	    verbose -log $comp_output
 	    set result [go_load "$output_file" "" ""]
@@ -610,7 +619,7 @@ 
 	    set dg-do-what-default "link"
 	    set output_file "./[file rootname [file tail $test]].exe"
 	    set comp_output [go_target_compile "$ofile1 $ofile2 $ofile3" \
-				 $output_file "executable" "$DEFAULT_GOCFLAGS"]
+				 $output_file "executable" "$options"]
 	    set comp_output [go-dg-prune $target_triplet $comp_output]
 	    if [string match "" $comp_output] {
 		pass $name
@@ -633,7 +642,7 @@ 
 	    set ofile2 "[file rootname [file tail $test]].o"
 	    set output_file "./[file rootname [file tail $test]].exe"
 	    set comp_output [go_target_compile "$ofile1 $ofile2" \
-				 $output_file "executable" "$DEFAULT_GOCFLAGS"]
+				 $output_file "executable" "$options"]
 	    set comp_output [go-dg-prune $target_triplet $comp_output]
 	    if [string match "" $comp_output] {
 		set result [go_load "$output_file" "" ""]
@@ -660,7 +669,7 @@ 
 	    set dg-do-what-default "link"
 	    set output_file "./[file rootname [file tail $file2]].exe"
 	    set comp_output [go_target_compile "$ofile1 $ofile2" \
-				 $output_file "executable" "$DEFAULT_GOCFLAGS"]
+				 $output_file "executable" "$options"]
 	    set comp_output [go-dg-prune $target_triplet $comp_output]
 	    if [string match "" $comp_output] {
 		set result [go_load "$output_file" "" ""]
@@ -726,7 +735,7 @@ 
 	    errchk $file3 ""
 	    set output_file "./[file rootname [file tail $test]].exe"
 	    set comp_output [go_target_compile "$ofile0 $ofile1 $ofile2" \
-				 $output_file "executable" "$DEFAULT_GOCFLAGS"]
+				 $output_file "executable" "$options"]
 	    set comp-output [go-dg-prune $target_triplet $comp_output]
 	    if [string match "" $comp_output] {
 		set result [go_load "$output_file" "" ""]
@@ -759,7 +768,7 @@ 
 	    set ofile2 "[file rootname [file tail $test]].o"
 	    set output_file "./[file rootname [file tail $test]].exe"
 	    set comp_output [go_target_compile "$ofile1 $ofile2" \
-				 $output_file "executable" "$DEFAULT_GOCFLAGS"]
+				 $output_file "executable" "$options"]
 	    set comp_output [go-dg-prune $target_triplet $comp_output]
 	    if [string match "" $comp_output] {
 		set result [go_load "$output_file" "" ""]
@@ -775,7 +784,7 @@ 
 	    regsub "/\[^/\]*$" $test "/cmplxdivide1.go" test2
 	    set output_file "./[file rootname [file tail $test]].o"
 	    set comp_output [go_target_compile "$test $test2" \
-			     $output_file "executable" "$DEFAULT_GOCFLAGS"]
+				 $output_file "executable" "$options"]
 	    set comp_output [go-dg-prune $target_triplet $comp_output]
 	    if [string match "" $comp_output] {
 		set result [go_load "$output_file" "" ""]
@@ -853,7 +862,7 @@ 
 	    set ofile2 "[file rootname [file tail $file2]].o"
 	    set output_file "./[file rootname [file tail $test]].exe"
 	    set comp_output [go_target_compile "$ofile1 $ofile2" \
-				 $output_file "executable" "$DEFAULT_GOCFLAGS"]
+				 $output_file "executable" "$options"]
 	    set comp_output [go-dg-prune $target_triplet $comp_output]
 	    if [string match "" $comp_output] {
 		setup_xfail "*-*-*"