[pushed] S390: Use soft float in s390-tdbregs test case

[pushed] S390: Use soft float in s390-tdbregs test case

[Andreas Arnez]

The GDB test case s390-tdbregs.exp verifies GDB's handling of the
"transaction diagnostic block".  For simplicity, the test case uses the
"transaction begin" (TBEGIN) instruction with the "allow floating-point
operation" flag set to zero.  But some GCC versions may indeed emit
floating point or vector instructions for this test case.  If this happens
in the transaction, it aborts, and an endless loop results.

This change tells the compiler to produce a soft-float binary, so no
floating-point or vector registers are touched.

gdb/testsuite/ChangeLog:

	* gdb.arch/s390-tdbregs.exp: Add the compile option -msoft-float.
---
 gdb/testsuite/ChangeLog                 | 4 ++++
 gdb/testsuite/gdb.arch/s390-tdbregs.exp | 5 ++++-
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index 210c078..c3fc9c3 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,7 @@
+2018-01-18  Andreas Arnez  <arnez@linux.vnet.ibm.com>
+
+	* gdb.arch/s390-tdbregs.exp: Add the compile option -msoft-float.
+
 2018-01-17  Mike Gulick  <mgulick@mathworks.com>
 
 	PR gdb/16577
diff --git a/gdb/testsuite/gdb.arch/s390-tdbregs.exp b/gdb/testsuite/gdb.arch/s390-tdbregs.exp
index 53d6c50..47d9d38 100644
--- a/gdb/testsuite/gdb.arch/s390-tdbregs.exp
+++ b/gdb/testsuite/gdb.arch/s390-tdbregs.exp
@@ -26,7 +26,10 @@ if { ![istarget s390-*-*] && ![istarget s390x-*-* ] } {
 
 standard_testfile .c
 
-if { [prepare_for_testing "failed to prepare" $testfile $srcfile] } {
+# Use soft float, so the compiler doesn't use floating-point or vector
+# instructions.
+if { [prepare_for_testing "failed to prepare" $testfile $srcfile \
+	  [list "debug" "additional_flags=-msoft-float"]] } {
     return -1
 }
 
-- 
2.5.0

Re: [pushed] S390: Use soft float in s390-tdbregs test case

[Pedro Alves]

On 01/18/2018 06:46 PM, Andreas Arnez wrote:
> The GDB test case s390-tdbregs.exp verifies GDB's handling of the
> "transaction diagnostic block".  For simplicity, the test case uses the
> "transaction begin" (TBEGIN) instruction with the "allow floating-point
> operation" flag set to zero.  But some GCC versions may indeed emit
> floating point or vector instructions for this test case.  If this happens
> in the transaction, it aborts, and an endless loop results.
> 
> This change tells the compiler to produce a soft-float binary, so no
> floating-point or vector registers are touched.

I found the rationale above quite instructive.  How about
putting it in the exp file directly?

>  
> -if { [prepare_for_testing "failed to prepare" $testfile $srcfile] } {
> +# Use soft float, so the compiler doesn't use floating-point or vector
> +# instructions.
> +if { [prepare_for_testing "failed to prepare" $testfile $srcfile \
> +	  [list "debug" "additional_flags=-msoft-float"]] } {
>      return -1

I mean, the comment here alone as is doesn't explain why we'd rather
the compiler not use float-point/vector insns, which I think is a
question people reading the testcase will tend to ask themselves,
and the answer isn't obvious.  At least to me.

Thanks,
Pedro Alves

Re: [pushed] S390: Use soft float in s390-tdbregs test case

[Andreas Arnez]

On Thu, Jan 18 2018, Pedro Alves wrote:

> On 01/18/2018 06:46 PM, Andreas Arnez wrote:
>> The GDB test case s390-tdbregs.exp verifies GDB's handling of the
>> "transaction diagnostic block".  For simplicity, the test case uses the
>> "transaction begin" (TBEGIN) instruction with the "allow floating-point
>> operation" flag set to zero.  But some GCC versions may indeed emit
>> floating point or vector instructions for this test case.  If this happens
>> in the transaction, it aborts, and an endless loop results.
>> 
>> This change tells the compiler to produce a soft-float binary, so no
>> floating-point or vector registers are touched.
>
> I found the rationale above quite instructive.  How about
> putting it in the exp file directly?
>
>>  
>> -if { [prepare_for_testing "failed to prepare" $testfile $srcfile] } {
>> +# Use soft float, so the compiler doesn't use floating-point or vector
>> +# instructions.
>> +if { [prepare_for_testing "failed to prepare" $testfile $srcfile \
>> +	  [list "debug" "additional_flags=-msoft-float"]] } {
>>      return -1
>
> I mean, the comment here alone as is doesn't explain why we'd rather
> the compiler not use float-point/vector insns, which I think is a
> question people reading the testcase will tend to ask themselves,
> and the answer isn't obvious.  At least to me.

Yeah, I guess it isn't.  In general, the test case was a bit
under-documented.  Thus, while at it, I'm adding some more information
as well.  So I pushed the patch below.  Thanks for the suggestion!

--
Andreas

-- >8 --
Subject: [PATCH] S390: Improve comments for s390-tdbregs test case

This adds more explanation as to why the test case must be compiled with
the -msoft-float option.  It also documents the my_tbegin and my_tend
functions.

gdb/testsuite/ChangeLog:

	* gdb.arch/s390-tdbregs.c (my_tbegin): Add comment documenting the
	function.
	(my_tend): Likewise.
	* gdb.arch/s390-tdbregs.exp: Enhance comment; explain the
	rationale of avoiding FP- and vector instructions.
---
 gdb/testsuite/ChangeLog                 | 8 ++++++++
 gdb/testsuite/gdb.arch/s390-tdbregs.c   | 9 +++++++++
 gdb/testsuite/gdb.arch/s390-tdbregs.exp | 6 ++++--
 3 files changed, 21 insertions(+), 2 deletions(-)

diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index f3d31e7..d510b79 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,11 @@
+2018-01-19  Andreas Arnez  <arnez@linux.vnet.ibm.com>
+
+	* gdb.arch/s390-tdbregs.c (my_tbegin): Add comment documenting the
+	function.
+	(my_tend): Likewise.
+	* gdb.arch/s390-tdbregs.exp: Enhance comment; explain the
+	rationale of avoiding FP- and vector instructions.
+
 2018-01-19  Ruslan Kabatsayev  <b7.10110111@gmail.com>
 
 	* gdb.arch/powerpc-d128-regs.exp: Replace expected "\[\t\]*" from
diff --git a/gdb/testsuite/gdb.arch/s390-tdbregs.c b/gdb/testsuite/gdb.arch/s390-tdbregs.c
index 2c768bb..57365ae 100644
--- a/gdb/testsuite/gdb.arch/s390-tdbregs.c
+++ b/gdb/testsuite/gdb.arch/s390-tdbregs.c
@@ -17,6 +17,13 @@
 
 #include <stdio.h>
 
+/* Start a transaction.  To avoid the need for FPR save/restore, assume
+   that no FP- or vector registers are modified within the transaction.
+   Thus invoke TBEGIN with the "allow floating-point operation" flag set
+   to zero, which forces a transaction abort when hitting an FP- or vector
+   instruction.  Also assume that TBEGIN will eventually succeed, so just
+   retry indefinitely.  */
+
 static void
 my_tbegin ()
 {
@@ -28,6 +35,8 @@ my_tbegin ()
       : "cc", "memory" );
 }
 
+/* End a transaction.  */
+
 static void
 my_tend ()
 {
diff --git a/gdb/testsuite/gdb.arch/s390-tdbregs.exp b/gdb/testsuite/gdb.arch/s390-tdbregs.exp
index 47d9d38..e454feb 100644
--- a/gdb/testsuite/gdb.arch/s390-tdbregs.exp
+++ b/gdb/testsuite/gdb.arch/s390-tdbregs.exp
@@ -26,8 +26,10 @@ if { ![istarget s390-*-*] && ![istarget s390x-*-* ] } {
 
 standard_testfile .c
 
-# Use soft float, so the compiler doesn't use floating-point or vector
-# instructions.
+# The test case assumes that no FP- or vector instructions occur within
+# the transaction.  Thus tell the compiler to use soft float, so it
+# doesn't emit them.  Some GCC versions may otherwise do so, and an
+# endless loop would result.
 if { [prepare_for_testing "failed to prepare" $testfile $srcfile \
 	  [list "debug" "additional_flags=-msoft-float"]] } {
     return -1
-- 
2.5.0

Re: [pushed] S390: Use soft float in s390-tdbregs test case

[Pedro Alves]

On 01/19/2018 01:20 PM, Andreas Arnez wrote:

> Yeah, I guess it isn't.  In general, the test case was a bit
> under-documented.  Thus, while at it, I'm adding some more information
> as well.  So I pushed the patch below.  Thanks for the suggestion!

Perfect!

Thanks,
Pedro Alves