Re: [PATCH] S390: Fix PR/77822.

2016-11-11 Thread Andreas Krebbel
On 11/11/2016 10:42 AM, Matthias Klose wrote:
> On 11.11.2016 09:58, Andreas Krebbel wrote:
>> On 11/08/2016 03:38 PM, Dominik Vogt wrote:
>>> The attached patch fixes PR/77822 on s390/s390x dor gcc-6 *only*.
>>> See https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77822
>>>
>>> Bootstrapped and regression tested on s390 and s390x biarch on a
>>> zEC12.
>>>
>>> For gcc-7, there will be a different patch.
>>
>> Applied to GCC 6 branch.  Thanks!
>> Please remember adding the PR number to the changelog entries to trigger 
>> bugzilla adding a comment
>> to the PR.
>>
>> As discussed offlist the range check for the position operand could be moved 
>> to a predicate.  This
>> will be part of the GCC head patch.
>>
>> I've just noticed that I had such checks already for the insv patterns and 
>> have added one to the
>> expander as well later. So for zero_extract as target operand this appeared 
>> to be a problem even
>> before GCC 6.
> 
> the gcc-6-branch now has the ChangeLog entry for gcc.target/s390/pr77822.c but
> not the test case.
> 
> Matthias
> 
Fixed.



Re: [PATCH] S390: Fix PR/77822.

2016-11-11 Thread Matthias Klose
On 11.11.2016 09:58, Andreas Krebbel wrote:
> On 11/08/2016 03:38 PM, Dominik Vogt wrote:
>> The attached patch fixes PR/77822 on s390/s390x dor gcc-6 *only*.
>> See https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77822
>>
>> Bootstrapped and regression tested on s390 and s390x biarch on a
>> zEC12.
>>
>> For gcc-7, there will be a different patch.
> 
> Applied to GCC 6 branch.  Thanks!
> Please remember adding the PR number to the changelog entries to trigger 
> bugzilla adding a comment
> to the PR.
> 
> As discussed offlist the range check for the position operand could be moved 
> to a predicate.  This
> will be part of the GCC head patch.
> 
> I've just noticed that I had such checks already for the insv patterns and 
> have added one to the
> expander as well later. So for zero_extract as target operand this appeared 
> to be a problem even
> before GCC 6.

the gcc-6-branch now has the ChangeLog entry for gcc.target/s390/pr77822.c but
not the test case.

Matthias



Re: [PATCH] S390: Fix PR/77822.

2016-11-11 Thread Andreas Krebbel
On 11/08/2016 03:38 PM, Dominik Vogt wrote:
> The attached patch fixes PR/77822 on s390/s390x dor gcc-6 *only*.
> See https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77822
> 
> Bootstrapped and regression tested on s390 and s390x biarch on a
> zEC12.
> 
> For gcc-7, there will be a different patch.

Applied to GCC 6 branch.  Thanks!
Please remember adding the PR number to the changelog entries to trigger 
bugzilla adding a comment
to the PR.

As discussed offlist the range check for the position operand could be moved to 
a predicate.  This
will be part of the GCC head patch.

I've just noticed that I had such checks already for the insv patterns and have 
added one to the
expander as well later. So for zero_extract as target operand this appeared to 
be a problem even
before GCC 6.

Bye,

-Andreas-



Re: [PATCH] S390: Fix PR/77822.

2016-11-09 Thread Dominik Vogt
On Tue, Nov 08, 2016 at 05:46:04PM +0100, Matthias Klose wrote:
> On 08.11.2016 15:38, Dominik Vogt wrote:
> > The attached patch fixes PR/77822 on s390/s390x dor gcc-6 *only*.
> > See https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77822
> > 
> > Bootstrapped and regression tested on s390 and s390x biarch on a
> > zEC12.
> 
> missing the testcase mentioned in the ChangeLog.

Sorry, patch with the missing file attached.

Ciao

Dominik ^_^  ^_^

-- 

Dominik Vogt
IBM Germany
gcc/ChangeLog

* config/s390/s390.md ("extzv", "*extzv_zEC12")
("*extzv_z10"): Check validity of zero_extend arguments.
gcc/testsuite/ChangeLog

* gcc.target/s390/pr77822.c: New test for PR/77822.
>From ce5616dca250c4735105346111e5ea7934b49b55 Mon Sep 17 00:00:00 2001
From: Dominik Vogt 
Date: Tue, 8 Nov 2016 09:54:03 +0100
Subject: [PATCH] S390: Fix PR/77822.

Check the range of the arguments in extzv patterns.  This avoids generating
risbg instructions with an out of range shift count.
---
 gcc/config/s390/s390.md |  12 +-
 gcc/testsuite/gcc.target/s390/pr77822.c | 307 
 2 files changed, 317 insertions(+), 2 deletions(-)
 create mode 100644 gcc/testsuite/gcc.target/s390/pr77822.c

diff --git a/gcc/config/s390/s390.md b/gcc/config/s390/s390.md
index b3d4370..899ed62 100644
--- a/gcc/config/s390/s390.md
+++ b/gcc/config/s390/s390.md
@@ -3708,6 +3708,10 @@
  (clobber (reg:CC CC_REGNUM))])]
   "TARGET_Z10"
 {
+  if (!IN_RANGE (INTVAL (operands[3]), 0, GET_MODE_BITSIZE (DImode) - 1)
+  || !IN_RANGE (INTVAL (operands[3]) + INTVAL (operands[2]), 1,
+   GET_MODE_BITSIZE (DImode)))
+FAIL;
   /* Starting with zEC12 there is risbgn not clobbering CC.  */
   if (TARGET_ZEC12)
 {
@@ -3726,7 +3730,9 @@
 (match_operand:GPR 1 "register_operand" "d")
 (match_operand 2 "const_int_operand" "")   ; size
 (match_operand 3 "const_int_operand" "")))] ; start]
-  "TARGET_ZEC12"
+  "TARGET_ZEC12
+   && IN_RANGE (INTVAL (operands[3]), 0, GET_MODE_BITSIZE (DImode) - 1)
+   && IN_RANGE (INTVAL (operands[3]) + INTVAL (operands[2]), 1, 
GET_MODE_BITSIZE (DImode))"
   "risbgn\t%0,%1,64-%2,128+63,+%3+%2" ; dst, src, start, end, shift
   [(set_attr "op_type" "RIE")])
 
@@ -3737,7 +3743,9 @@
(match_operand 2 "const_int_operand" "")   ; size
(match_operand 3 "const_int_operand" ""))) ; start
(clobber (reg:CC CC_REGNUM))]
-  "TARGET_Z10"
+  "TARGET_Z10
+   && IN_RANGE (INTVAL (operands[3]), 0, GET_MODE_BITSIZE (DImode) - 1)
+   && IN_RANGE (INTVAL (operands[3]) + INTVAL (operands[2]), 1, 
GET_MODE_BITSIZE (DImode))"
   "risbg\t%0,%1,64-%2,128+63,+%3+%2" ; dst, src, start, end, shift
   [(set_attr "op_type" "RIE")
(set_attr "z10prop" "z10_super_E1")])
diff --git a/gcc/testsuite/gcc.target/s390/pr77822.c 
b/gcc/testsuite/gcc.target/s390/pr77822.c
new file mode 100644
index 000..6789152
--- /dev/null
+++ b/gcc/testsuite/gcc.target/s390/pr77822.c
@@ -0,0 +1,307 @@
+/* This testcase checks that the shift operand of r*sbg instructions is in
+   range.  */
+
+/* { dg-do compile } */
+/* { dg-options "-O3 -march=zEC12 -Wno-shift-count-overflow" } */
+
+int g;
+
+void pos_ll_129 (long long b)
+{
+  if (b >> 129 & 1)
+g = b;
+}
+
+void sizepos_ll_134 (long long b)
+{
+  if (b >> 134 & 1)
+g = b;
+}
+
+void pos_ll_65 (long long b)
+{
+  if (b >> 65 & 1)
+g = b;
+}
+
+void sizepos_ll_70 (long long b)
+{
+  if (b >> 70 & 1)
+g = b;
+}
+
+void pos_ll_33 (long long b)
+{
+  if (b >> 33 & 1)
+g = b;
+}
+
+void sizepos_ll_38 (long long b)
+{
+  if (b >> 38 & 1)
+g = b;
+}
+
+void pos_ll_17 (long long b)
+{
+  if (b >> 17 & 1)
+g = b;
+}
+
+void sizepos_ll_22 (long long b)
+{
+  if (b >> 22 & 1)
+g = b;
+}
+
+void pos_ll_8 (long long b)
+{
+  if (b >> 8 & 1)
+g = b;
+}
+
+void sizepos_ll_13 (long long b)
+{
+  if (b >> 13 & 1)
+g = b;
+}
+
+void pos_l_129 (long b)
+{
+  if (b >> 129 & 1)
+g = b;
+}
+
+void sizepos_l_134 (long b)
+{
+  if (b >> 134 & 1)
+g = b;
+}
+
+void pos_l_65 (long b)
+{
+  if (b >> 65 & 1)
+g = b;
+}
+
+void sizepos_l_70 (long b)
+{
+  if (b >> 70 & 1)
+g = b;
+}
+
+void pos_l_33 (long b)
+{
+  if (b >> 33 & 1)
+g = b;
+}
+
+void sizepos_l_38 (long b)
+{
+  if (b >> 38 & 1)
+g = b;
+}
+
+void pos_l_17 (long b)
+{
+  if (b >> 17 & 1)
+g = b;
+}
+
+void sizepos_l_22 (long b)
+{
+  if (b >> 22 & 1)
+g = b;
+}
+
+void pos_l_8 (long b)
+{
+  if (b >> 8 & 1)
+g = b;
+}
+
+void sizepos_l_13 (long b)
+{
+  if (b >> 13 & 1)
+g = b;
+}
+
+void pos_i_129 (int b)
+{
+  if (b >> 129 & 1)
+g = b;
+}
+
+void sizepos_i_134 (int b)
+{
+  if (b >> 134 & 1)
+g = b;
+}
+
+void pos_i_65 (int b)
+{
+  if (b >> 65 & 1)
+g = b;
+}
+
+void sizepos_i_70 (int b)
+{
+  if (b >> 70 & 1)
+g = b;
+}
+
+void pos_i_33 (int b)
+{
+  if (b >> 33 & 1)
+g = b;
+}
+
+void sizepos_i_38 (int b)
+{
+  if (b >> 38 & 1)
+g = b;
+}

Re: [PATCH] S390: Fix PR/77822.

2016-11-08 Thread Matthias Klose
On 08.11.2016 15:38, Dominik Vogt wrote:
> The attached patch fixes PR/77822 on s390/s390x dor gcc-6 *only*.
> See https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77822
> 
> Bootstrapped and regression tested on s390 and s390x biarch on a
> zEC12.

missing the testcase mentioned in the ChangeLog.