On Thu, Jan 26, 2017 at 05:11:27PM +0100, Dominik Vogt wrote:
> The patch fixes the s390x crash reported in PR 79240:
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=79240
> Regression tested and bootstrapped on s390x biarch and s390.
> gcc/ChangeLog-pr79240
>
> PR target/79240
> * config/s390/s390.c (s390_extzv_shift_ok): Returns false, don't assert.
> * config/s390/s390.md ("*r<noxa>sbg_<mode>_srl_bitmask")
> ("*r<noxa>sbg_<mode>_sll_bitmask")
> ("*extzv_<mode>_srl<clobbercc_or_nocc>")
> ("*extzv_<mode>_sll<clobbercc_or_nocc>"):
> Use contiguous_bitmask_nowrap_operand
> gcc/testsuite/ChangeLog-pr79240
>
> * gcc.target/s390/pr79240.c: New test.
> diff --git a/gcc/config/s390/s390.c b/gcc/config/s390/s390.c
> index fe65846..b1dfbfa 100644
> --- a/gcc/config/s390/s390.c
> +++ b/gcc/config/s390/s390.c
> @@ -2472,7 +2472,8 @@ s390_extzv_shift_ok (int bitsize, int rotl, unsigned
> HOST_WIDE_INT contig)
> bool ok;
>
> ok = s390_contiguous_bitmask_nowrap_p (contig, bitsize, &start, &end);
> - gcc_assert (ok);
> + if (!ok)
> + return false;
If the predicates are supposed to ensure it, then I think the assert is
fine.
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/s390/pr79240.c
> @@ -0,0 +1,12 @@
> +/* This testcase checks that s390_extzv_shift_ok does not cause an assertion
> + failure. */
> +
> +/* { dg-do compile } */
> +/* { dg-options "-w -march=z196 -mtune=zEC12 -m64 -mzarch -O2" } */
> +
> +a, b;
> +c ()
> +{
> + int d = sizeof (int) * a + 16 - a * sizeof (int) % 16;
> + b = d;
> +}
Don't want to nitpick too much, but I think it would be nice not to throw in
all the garbage creduce creates, so e.g. the omitted int or function
implicitly returning int but without return don't look very nice (of course,
unless that is something the test is looking for).
See the slightly cleaned up testcase in the PR.
I also wonder about whether -m64 is desirable, e.g. gcc.target/i386/ has
clear rule that -m32/-m64 should be avoided, either the test should work
fine with both -m32 and -m64 (this case), then there is no reason to add it,
or not, then it is better to guard the test with lp64 or ilp32 or similar
effective target. I think the -mtune=/-mzarch aren't needed either.
There is nothing s390 specific on the test except for the -march=z196 or
-march=z10 or similar, so you might as well just make it a gcc.dg/
compile test with
/* { dg-options "-O2" } */
/* { dg-additional-options "-march=z196" { target s390*-*-* } } */
or so.
Jakub
