diff mbox

Several regressions and we branch soon.

Message ID 87h9ppkr0d.fsf@br87z6lw.de.ibm.com
State New
Headers show

Commit Message

Andreas Arnez June 30, 2015, 6:09 p.m. UTC
On Tue, Jun 30 2015, Yao Qi wrote:

> Andreas Arnez <arnez@linux.vnet.ibm.com> writes:
>
>> Right, this was considered in the patch.  But only what I've actually
>> seen fail in my testing is marked as KFAIL.  Since more FAILs are
>> observed now, maybe we could mark them as KFAIL as well.  Or skip them
>> altogether, like in the patch below.  WDYT?
>
> I am inclined to skip them altogether, but I think we need skip more.
> With your patch applied,  I still see them in gdb.sum
>
>  KFAIL: gdb.base/gnu_vector.exp: call add_some_intvecs (PRMS: gdb/18537)
>  KPASS: gdb.base/gnu_vector.exp: call add_many_charvecs (PRMS gdb/18537)
>  KFAIL: gdb.base/gnu_vector.exp: call add_various_floatvecs (PRMS: gdb/18537)
>  KFAIL: gdb.base/gnu_vector.exp: call add_structvecs (PRMS: gdb/18537)
>  KFAIL: gdb.base/gnu_vector.exp: skip remaining vector ABI tests on this arch (PRMS: gdb/18537)
>
> KPASS is confusing here.  I'd like to skip all of them on x86 and emit
> UNSUPPORTED in gdb.sum, because we've already know that vector infcall
> doesn't support on x86, UNSUPPORTED is better than KFAIL, IMO.

You're probably right.  I chose KFAIL because GDB doesn't provide any
indication that this is not expected to work correctly, but yields a
wrong result instead.  Anyway, the KFAILs obviously bring more trouble
than benefit, and UNSUPPORTED looks good enough to me.

So how about this?

-- >8 --
Subject: [PATCH] gnu_vector.exp: Skip infcall tests on x86/x86_64

Since the new KFAILs/KPASSs for the infcall tests on x86 and x86_64
targets generated unnecessary noise, this change skips them with
UNSUPPORTED instead.

gdb/testsuite/ChangeLog:

	* gdb.base/gnu_vector.exp: On x86 and x86_64 targets, skip the
	infcall tests instead of setting up for KFAIL.
---
 gdb/testsuite/gdb.base/gnu_vector.exp |   13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

Comments

Yao Qi July 1, 2015, 8 a.m. UTC | #1
On 30/06/15 19:09, Andreas Arnez wrote:
> So how about this?
>
> -- >8 --
> Subject: [PATCH] gnu_vector.exp: Skip infcall tests on x86/x86_64
>
> Since the new KFAILs/KPASSs for the infcall tests on x86 and x86_64
> targets generated unnecessary noise, this change skips them with
> UNSUPPORTED instead.
>
> gdb/testsuite/ChangeLog:
>
> 	* gdb.base/gnu_vector.exp: On x86 and x86_64 targets, skip the
> 	infcall tests instead of setting up for KFAIL.

The patch is OK to me.
Yao Qi July 10, 2015, 9:33 a.m. UTC | #2
Andreas Arnez <arnez@linux.vnet.ibm.com> writes:

> Subject: [PATCH] gnu_vector.exp: Skip infcall tests on x86/x86_64
>
> Since the new KFAILs/KPASSs for the infcall tests on x86 and x86_64
> targets generated unnecessary noise, this change skips them with
> UNSUPPORTED instead.

Hi Andreas,
I still see some fails in gnu_vector.exp in various architectures,

Here are some fails on ppc64be-m64, as I found from buildbot
https://www.sourceware.org/ml/gdb-testers/2015-q3/msg01198.html

new FAIL: gdb.base/gnu_vector.exp: call add_some_intvecs
new FAIL: gdb.base/gnu_vector.exp: call add_various_floatvecs
new FAIL: gdb.base/gnu_vector.exp: finish shows vector return value
new FAIL: gdb.base/gnu_vector.exp: verify vector return value (the program exited)

Does GDB support vector infcall on ppc64be?  I checked the test result
on ppc64le https://www.sourceware.org/ml/gdb-testers/2015-q3/msg01201.html
but gnu_vector.exp isn't compiled successfully (due to old gcc?)  this
case isn't compiled successfully on aix buildslave either.

I also see two fails on s390x from buildbot
https://www.sourceware.org/ml/gdb-testers/2015-q3/msg00957.html

FAIL: gdb.base/gnu_vector.exp: finish shows vector return value
FAIL: gdb.base/gnu_vector.exp: verify vector return value (the program exited)

IIUC, vector infcall should be supported on s390 GDB, right?

I also see some vector infcall fails on both arm and aarch64 too.  What
GDB targets should support vector infcall?  ppc64 (le and be) and s390?
I am wondering we should only do the vector infcall tests on the
supported GDB targets, and skip for the rest of them.
Andreas Arnez July 10, 2015, 4:12 p.m. UTC | #3
On Fri, Jul 10 2015, Yao Qi wrote:

> Andreas Arnez <arnez@linux.vnet.ibm.com> writes:
>
>> Subject: [PATCH] gnu_vector.exp: Skip infcall tests on x86/x86_64
>>
>> Since the new KFAILs/KPASSs for the infcall tests on x86 and x86_64
>> targets generated unnecessary noise, this change skips them with
>> UNSUPPORTED instead.
>
> Hi Andreas,
> I still see some fails in gnu_vector.exp in various architectures,
>
> Here are some fails on ppc64be-m64, as I found from buildbot
> https://www.sourceware.org/ml/gdb-testers/2015-q3/msg01198.html
>
> new FAIL: gdb.base/gnu_vector.exp: call add_some_intvecs
> new FAIL: gdb.base/gnu_vector.exp: call add_various_floatvecs
> new FAIL: gdb.base/gnu_vector.exp: finish shows vector return value
> new FAIL: gdb.base/gnu_vector.exp: verify vector return value (the program exited)
>
> Does GDB support vector infcall on ppc64be?

AFAIK, it should.  However, on that system the compilation with
"-mcpu=native" fails because GCC5 emits an ABI warning:

  gnu_vector.c:62:1: note: the layout of aggregates containing vectors with 4-byte alignment has changed in GCC 5

Then the test case falls back to compiling without an "-mcpu=" flag, so
the FAILs occur with GCC's default machine options.  I am not sure
whether that is supposed to work with GDB.  (Does anybody know?)

Anyway, maybe we should add "-Wno-psabi" to the compile options.  This
will likely get rid of the (probably unimportant) FAILs above.

> I checked the test result
> on ppc64le https://www.sourceware.org/ml/gdb-testers/2015-q3/msg01201.html
> but gnu_vector.exp isn't compiled successfully (due to old gcc?)  this
> case isn't compiled successfully on aix buildslave either.

GCC5 on ppc64le emits the ABI warning even in absence of "-mcpu=".
Again it should help to add "-Wno-psabi".

>
> I also see two fails on s390x from buildbot
> https://www.sourceware.org/ml/gdb-testers/2015-q3/msg00957.html
>
> FAIL: gdb.base/gnu_vector.exp: finish shows vector return value
> FAIL: gdb.base/gnu_vector.exp: verify vector return value (the program exited)
>
> IIUC, vector infcall should be supported on s390 GDB, right?

Yes.  But in this case no vector ABI is used, because that test machine
does not have a vector facility and because -march=native is not
supported by GCC (yet).  Thus vector return values are not passed in
vector registers, but according to RETURN_VALUE_STRUCT_CONVENTION.  And
then we hit the problem that displaying such return values is not
supported by GDB: https://sourceware.org/bugzilla/show_bug.cgi?id=8549

Note that various other testsuite FAILs on s390 are due to
non-displayable structure return values as well, e.g.:

  FAIL: gdb.ada/array_return.exp: value printed by finish of Create_Small

Thus I have already been working on fixing that.

> I also see some vector infcall fails on both arm and aarch64 too.  What
> GDB targets should support vector infcall?  ppc64 (le and be) and s390?

Sorry, I do not know.  This question should better be addressed to the
various architecture maintainers.  Note that many architectures have
*multiple* vector ABIs, depending on the level of hardware support
available.  So a complete answer to your question would be a filled-out
table like this (where the examples are obviously completely made up):

  | architecture | vector ABI | infcall | "finish" |
  |--------------+------------+---------+----------|
  | foo          | no HW      | OK      | bad ret  |
  |              | VX16       | broken  | broken   |
  |              | VY32       | OK      | OK       |
  |--------------+------------+---------+----------|
  | bar          | no HW      | OK      | OK       |
  |              | VBLURB     | OK      | OK       |
  |              | V-ng       | unsupp  | no ret   |

The s390 part currently looks like this:

  | architecture | vector ABI          | infcall | "finish" |
  |--------------+---------------------+---------+----------|
  | s390x        | no HW               | OK      | no ret   |
  |              | S390_VECTOR_ABI_128 | OK      | OK       |

(1) Assuming that the vector return value fits in a vector register.

> I am wondering we should only do the vector infcall tests on the
> supported GDB targets, and skip for the rest of them.

We could.  On the other hand there is a difference from the usual
"lacking support" case: Normally GDB tells the user about the lacking
support.  Here, GDB performs a bogus inferior function call instead,
shows a wrong return value, or even crashes the inferior.  This seems
more like a bug than a missing feature to me.  In my view, targets that
can not perform vector ABI infcalls correctly should at least suppress
the infcall and emit an appropriate error message.

--
Andreas
diff mbox

Patch

diff --git a/gdb/testsuite/gdb.base/gnu_vector.exp b/gdb/testsuite/gdb.base/gnu_vector.exp
index cf91fbb..173da4d 100644
--- a/gdb/testsuite/gdb.base/gnu_vector.exp
+++ b/gdb/testsuite/gdb.base/gnu_vector.exp
@@ -177,16 +177,19 @@  gdb_test "ptype struct_with_vector_1" "type = struct {\r\n\[\t \]+int i;\r\n\[\t
 
 # Test inferior function calls with vector arguments and/or vector
 # return values.
-setup_kfail gdb/18537 "i?86-*-*" "x86_64-*-*"
+if { [istarget "i?86-*-*"] || [istarget "x86_64-*-*" ] } {
+    # These platforms don't support infcalls with vector arguments
+    # and/or vector return values, so skip the remaining tests.
+    # See also PR exp/18537.
+    unsupported "skip remaining vector ABI tests on this arch"
+    return
+}
 gdb_test "print add_some_intvecs(i4a, i4b, 3 * i4a)" "= \\{17, 34, 72, 132\\}" \
     "call add_some_intvecs"
-setup_kfail gdb/18537 "i?86-*-*" "x86_64-*-*"
 gdb_test "print add_many_charvecs(c4, c4, c4, c4, c4, c4, c4, c4, c4, c4)" \
     "= \\{10, 20, 30, 40\\}" "call add_many_charvecs"
-setup_kfail gdb/18537 "i?86-*-*" "x86_64-*-*"
 gdb_test "print add_various_floatvecs(2, f4a, f4b)" "= \\{3, 6, 16, 20\\}" \
     "call add_various_floatvecs"
-setup_kfail gdb/18537 "i?86-*-*" "x86_64-*-*"
 gdb_test "print add_structvecs(i2, (struct just_int2)\{2*i2\}, (struct two_int2)\{3*i2, 4*i2\})" \
     "= \\{i = \\{10, 20\\}\\}" "call add_structvecs"
 gdb_test "print add_singlevecs((char1) \{6\}, (int1) \{12\}, (double1) \{24\})" "= \\{42\\}" \
@@ -195,7 +198,6 @@  gdb_test "print add_singlevecs((char1) \{6\}, (int1) \{12\}, (double1) \{24\})"
 # Test vector return value handling with "finish" and "return".
 gdb_breakpoint "add_some_intvecs"
 gdb_continue "add_some_intvecs"
-setup_kfail gdb/18537 "i?86-*-*" "x86_64-*-*"
 gdb_test "finish" "Value returned is .* = \\{10, 20, 48, 72\\}" \
     "finish shows vector return value"
 gdb_continue "add_some_intvecs"
@@ -203,5 +205,4 @@  gdb_test "return (int4) \{4, 2, 7, 6\}" \
     "#0 .* main .*" \
     "set vector return value" \
     "Make add_some_intvecs return now. .y or n.*" "y"
-setup_kfail gdb/18537 "i?86-*-*" "x86_64-*-*"
 gdb_test "continue" "4 2 7 6\r\n.*" "verify vector return value"