diff mbox

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

Message ID m3k1we0yv3.fsf@oc1027705133.ibm.com
State New
Headers show

Commit Message

Andreas Arnez Jan. 19, 2018, 1:20 p.m. UTC
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(-)

Comments

Pedro Alves Jan. 19, 2018, 1:52 p.m. UTC | #1
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
diff mbox

Patch

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