diff mbox

[v2] S/390: PR target/79240: Fix assertion in s390_extzv_shift_ok.

Message ID 20170126204239.GA18640@linux.vnet.ibm.com
State New
Headers show

Commit Message

Dominik Vogt Jan. 26, 2017, 8:42 p.m. UTC 
On Thu, Jan 26, 2017 at 05:45:18PM +0100, Jakub Jelinek wrote:
> On Thu, Jan 26, 2017 at 05:43:13PM +0100, Dominik Vogt wrote:
> > > If the predicates are supposed to ensure it, then I think the assert is
> > > fine.
> > 
> > Is it guaranteed that the predicate conditions are evaluated
> > before executing the conditions?
> 
> Yes.  You can see it in insn-recog.c...

Updated patch attached.

changes:

  * Don't remove assertion.
  * Use simplified test case.

Bootstrapped and regression tested on a zEC12 with s390x biarch
and s390.

Ciao

Dominik ^_^  ^_^

Comments

Jakub Jelinek Jan. 26, 2017, 8:52 p.m. UTC | #1 
On Thu, Jan 26, 2017 at 09:42:39PM +0100, Dominik Vogt wrote:
> gcc/ChangeLog-pr79240
> 
> 	PR target/79240
> 	* 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

Missing full stop at the end.

> --- a/gcc/config/s390/s390.md
> +++ b/gcc/config/s390/s390.md
> @@ -4127,7 +4127,7 @@
>  	    (lshiftrt:GPR
>                (match_operand:GPR 1 "nonimmediate_operand" "d")
>                (match_operand:GPR 3 "nonzero_shift_count_operand" ""))
> -            (match_operand:GPR 2 "contiguous_bitmask_operand" ""))
> +            (match_operand:GPR 2 "contiguous_bitmask_nowrap_operand" ""))
>  	  (match_operand:GPR 4 "nonimmediate_operand" "0")))
>     (clobber (reg:CC CC_REGNUM))]
>    "TARGET_Z10

This shows another cleanup possibility for later (GCC8), for
(match_operand... "whatever_operand" "")
one can use just
(match_operand... "whatever_operand")
The empty constraint is implicit.  E.g. in i386/*.md Uros has done that some
time ago and keeps redoing it if some of these get in.
One can verify if it doesn't change anything by doing make mddump
before/after and comparing the tmp-mddump.md files.

	Jakub
Andreas Krebbel Jan. 30, 2017, 9:55 a.m. UTC | #2 
On 01/26/2017 09:42 PM, Dominik Vogt wrote:
> On Thu, Jan 26, 2017 at 05:45:18PM +0100, Jakub Jelinek wrote:
>> On Thu, Jan 26, 2017 at 05:43:13PM +0100, Dominik Vogt wrote:
>>>> If the predicates are supposed to ensure it, then I think the assert is
>>>> fine.
>>>
>>> Is it guaranteed that the predicate conditions are evaluated
>>> before executing the conditions?
>>
>> Yes.  You can see it in insn-recog.c...
> 
> Updated patch attached.
> 
> changes:
> 
>   * Don't remove assertion.
>   * Use simplified test case.
> 
> Bootstrapped and regression tested on a zEC12 with s390x biarch
> and s390.

Applied. Thanks!

-Andreas-
diff mbox

Patch

diff --git a/gcc/config/s390/s390.md b/gcc/config/s390/s390.md
index 3135175..e47c2e9 100644
--- a/gcc/config/s390/s390.md
+++ b/gcc/config/s390/s390.md
@@ -4127,7 +4127,7 @@ 
 	    (lshiftrt:GPR
               (match_operand:GPR 1 "nonimmediate_operand" "d")
               (match_operand:GPR 3 "nonzero_shift_count_operand" ""))
-            (match_operand:GPR 2 "contiguous_bitmask_operand" ""))
+            (match_operand:GPR 2 "contiguous_bitmask_nowrap_operand" ""))
 	  (match_operand:GPR 4 "nonimmediate_operand" "0")))
    (clobber (reg:CC CC_REGNUM))]
   "TARGET_Z10
@@ -4143,7 +4143,7 @@ 
 	    (ashift:GPR
               (match_operand:GPR 1 "nonimmediate_operand" "d")
               (match_operand:GPR 3 "nonzero_shift_count_operand" ""))
-            (match_operand:GPR 2 "contiguous_bitmask_operand" ""))
+            (match_operand:GPR 2 "contiguous_bitmask_nowrap_operand" ""))
 	  (match_operand:GPR 4 "nonimmediate_operand" "0")))
    (clobber (reg:CC CC_REGNUM))]
   "TARGET_Z10
@@ -7191,7 +7191,7 @@ 
 	(and:GPR (lshiftrt:GPR
 		   (match_operand:GPR 1 "register_operand" "d")
 		   (match_operand:GPR 2 "nonzero_shift_count_operand" ""))
-		(match_operand:GPR 3 "contiguous_bitmask_operand" "")))]
+		(match_operand:GPR 3 "contiguous_bitmask_nowrap_operand" "")))]
   "<z10_or_zEC12_cond>
    /* Note that even for the SImode pattern, the rotate is always DImode.  */
    && s390_extzv_shift_ok (<bitsize>, -INTVAL (operands[2]),
@@ -7205,7 +7205,7 @@ 
 	(and:GPR (ashift:GPR
 		  (match_operand:GPR 1 "register_operand" "d")
 		  (match_operand:GPR 2 "nonzero_shift_count_operand" ""))
-		(match_operand:GPR 3 "contiguous_bitmask_operand" "")))]
+		(match_operand:GPR 3 "contiguous_bitmask_nowrap_operand" "")))]
   "<z10_or_zEC12_cond>
    && s390_extzv_shift_ok (<bitsize>, INTVAL (operands[2]),
 			   INTVAL (operands[3]))"
diff --git a/gcc/testsuite/gcc.target/s390/pr79240.c b/gcc/testsuite/gcc.target/s390/pr79240.c
new file mode 100644
index 0000000..bd8f72f
--- /dev/null
+++ b/gcc/testsuite/gcc.target/s390/pr79240.c
@@ -0,0 +1,11 @@ 
+/* 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" } */
+
+int
+foo (int a)
+{
+  return sizeof (int) * a + 16 - a * sizeof (int) % 16;
+}