S/390: Fix warnings in "*setmem_long..." patterns.

S/390: Fix warnings in "*setmem_long..." patterns.

[Dominik Vogt]

[-- Attachment #1: Type: text/plain, Size: 682 bytes --]

The attached patch fixes some warnings generated by the setmem...
patterns in s390.md during build and add test cases for the
patterns.  The patch is to be added on to p of the movstr patch:
https://gcc.gnu.org/ml/gcc-patches/2015-11/msg03485.html

The test cases validate that the patterns are actually used, but
at the moment the setmem_long_and pattern is never actually used
and thus the test case would fail.  So I've split the patch in two
(both attached to this message) to activate this part of the test
once we've fixed that.

The patch has passed the SPEC2006 testsuite without any measurable
changes in performance.

Ciao

Dominik ^_^  ^_^

-- 

Dominik Vogt
IBM Germany

[-- Attachment #2: 0001-0001b-Changelog --]
[-- Type: text/plain, Size: 368 bytes --]

gcc/ChangeLog

	* config/s390/s390.c (s390_expand_setmem): Use new expanders.
	* config/s390/s390.md ("*setmem_long")
	("*setmem_long_and", "*setmem_long_31z"): Fix warnings.
	("setmem_long_<P:mode>"): New expanders.
	("setmem_long"): Removed.

gcc/testsuite/ChangeLog

	* gcc.target/s390/md/setmem_long-1.c: New test.
	* gcc.target/s390/md/setmem_long-2.c: New test.

[-- Attachment #3: 0001-S-390-Fix-warnings-in-setmem_long.-patterns.patch --]
[-- Type: text/x-diff, Size: 5264 bytes --]

From 6b484cd8a9f39a38b3e990b4ac160c8254c03f6b Mon Sep 17 00:00:00 2001
From: Dominik Vogt <vogt@linux.vnet.ibm.com>
Date: Wed, 4 Nov 2015 03:16:24 +0100
Subject: [PATCH 1/1.5] S/390: Fix warnings in "*setmem_long..." patterns.

---
 gcc/config/s390/s390.c                           |  7 ++++++-
 gcc/config/s390/s390.md                          | 18 +++++++++++++-----
 gcc/testsuite/gcc.target/s390/md/setmem_long-1.c | 20 ++++++++++++++++++++
 gcc/testsuite/gcc.target/s390/md/setmem_long-2.c | 20 ++++++++++++++++++++
 4 files changed, 59 insertions(+), 6 deletions(-)
 create mode 100644 gcc/testsuite/gcc.target/s390/md/setmem_long-1.c
 create mode 100644 gcc/testsuite/gcc.target/s390/md/setmem_long-2.c

diff --git a/gcc/config/s390/s390.c b/gcc/config/s390/s390.c
index 40ee2f7..8f2396f 100644
--- a/gcc/config/s390/s390.c
+++ b/gcc/config/s390/s390.c
@@ -5178,7 +5178,12 @@ s390_expand_setmem (rtx dst, rtx len, rtx val)
   else if (TARGET_MVCLE)
     {
       val = force_not_mem (convert_modes (Pmode, QImode, val, 1));
-      emit_insn (gen_setmem_long (dst, convert_to_mode (Pmode, len, 1), val));
+      if (TARGET_64BIT)
+	emit_insn (gen_setmem_long_di (dst, convert_to_mode (Pmode, len, 1),
+				       val));
+      else
+	emit_insn (gen_setmem_long_si (dst, convert_to_mode (Pmode, len, 1),
+				       val));
     }
 
   else
diff --git a/gcc/config/s390/s390.md b/gcc/config/s390/s390.md
index 75e9af7..ed98101 100644
--- a/gcc/config/s390/s390.md
+++ b/gcc/config/s390/s390.md
@@ -70,6 +70,9 @@
    ; Copy CC as is into the lower 2 bits of an integer register
    UNSPEC_CC_TO_INT
 
+   ; Convert Pmode to BLKmode
+   UNSPEC_P_TO_BLK
+
    ; GOT/PLT and lt-relative accesses
    UNSPEC_LTREL_OFFSET
    UNSPEC_LTREL_BASE
@@ -3281,11 +3284,12 @@
 
 ; Initialize a block of arbitrary length with (operands[2] % 256).
 
-(define_expand "setmem_long"
+(define_expand "setmem_long_<P:mode>"
   [(parallel
     [(clobber (match_dup 1))
      (set (match_operand:BLK 0 "memory_operand" "")
-          (match_operand 2 "shift_count_or_setmem_operand" ""))
+	  (unspec:BLK [(match_operand:P 2 "shift_count_or_setmem_operand" "")]
+		      UNSPEC_P_TO_BLK))
      (use (match_operand 1 "general_operand" ""))
      (use (match_dup 3))
      (clobber (reg:CC CC_REGNUM))])]
@@ -3312,7 +3316,8 @@
 (define_insn "*setmem_long"
   [(clobber (match_operand:<DBL> 0 "register_operand" "=d"))
    (set (mem:BLK (subreg:P (match_operand:<DBL> 3 "register_operand" "0") 0))
-        (match_operand 2 "shift_count_or_setmem_operand" "Y"))
+        (unspec:BLK [(match_operand:P 2 "shift_count_or_setmem_operand" "Y")]
+		UNSPEC_P_TO_BLK))
    (use (match_dup 3))
    (use (match_operand:<DBL> 1 "register_operand" "d"))
    (clobber (reg:CC CC_REGNUM))]
@@ -3324,7 +3329,9 @@
 (define_insn "*setmem_long_and"
   [(clobber (match_operand:<DBL> 0 "register_operand" "=d"))
    (set (mem:BLK (subreg:P (match_operand:<DBL> 3 "register_operand" "0") 0))
-        (and (match_operand 2 "shift_count_or_setmem_operand" "Y")
+        (and:BLK (unspec:BLK
+	      [(match_operand:P 2 "shift_count_or_setmem_operand" "Y")]
+	      UNSPEC_P_TO_BLK)
 	     (match_operand 4 "const_int_operand"             "n")))
    (use (match_dup 3))
    (use (match_operand:<DBL> 1 "register_operand" "d"))
@@ -3338,7 +3345,8 @@
 (define_insn "*setmem_long_31z"
   [(clobber (match_operand:TI 0 "register_operand" "=d"))
    (set (mem:BLK (subreg:SI (match_operand:TI 3 "register_operand" "0") 4))
-        (match_operand 2 "shift_count_or_setmem_operand" "Y"))
+        (unspec:BLK [(match_operand:P 2 "shift_count_or_setmem_operand" "Y")]
+		UNSPEC_P_TO_BLK))
    (use (match_dup 3))
    (use (match_operand:TI 1 "register_operand" "d"))
    (clobber (reg:CC CC_REGNUM))]
diff --git a/gcc/testsuite/gcc.target/s390/md/setmem_long-1.c b/gcc/testsuite/gcc.target/s390/md/setmem_long-1.c
new file mode 100644
index 0000000..9a926ce
--- /dev/null
+++ b/gcc/testsuite/gcc.target/s390/md/setmem_long-1.c
@@ -0,0 +1,20 @@
+/* Machine description pattern tests.  */
+
+/* { dg-do compile { target { lp64 } } } */
+/* { dg-options "-mmvcle -dP" } */
+
+#include <string.h>
+
+void test(char *p, char c)
+{
+  __builtin_memset (p, c, 10000);
+}
+
+void test2(char *p, int c)
+{
+  __builtin_memset (p, (char)c, 10000);
+}
+
+/* Check that the right patterns are used.  */
+/* { dg-final { scan-assembler-times "c:10 .\*\{\\*setmem_long\}" 1 } } */
+/* { dg-final { scan-assembler-times "c:15 .\*\{\\*setmem_long\}" 1 } } */
diff --git a/gcc/testsuite/gcc.target/s390/md/setmem_long-2.c b/gcc/testsuite/gcc.target/s390/md/setmem_long-2.c
new file mode 100644
index 0000000..ce6aa42
--- /dev/null
+++ b/gcc/testsuite/gcc.target/s390/md/setmem_long-2.c
@@ -0,0 +1,20 @@
+/* Machine description pattern tests.  */
+
+/* { dg-do compile { target { ! lp64 } } } */
+/* { dg-options "-mmvcle -mzarch -dP" } */
+
+#include <string.h>
+
+void test(char *p, char c)
+{
+  __builtin_memset (p, c, 10000);
+}
+
+void test2(char *p, int c)
+{
+  __builtin_memset (p, (char)c, 10000);
+}
+
+/* Check that the right patterns are used.  */
+/* { dg-final { scan-assembler-times "c:10 .\*\{\\*setmem_long_31z\}" 1 } } */
+/* { dg-final { scan-assembler-times "c:15 .\*\{\\*setmem_long_31z\}" 1 } } */
-- 
2.3.0


[-- Attachment #4: 0001b-S-390-Test-that-the-setmem_long_and-pattern-is-used.patch --]
[-- Type: text/x-diff, Size: 1991 bytes --]

From 7f9b0b46f49cb0c99dc461572057726d44d88e2e Mon Sep 17 00:00:00 2001
From: Dominik Vogt <vogt@linux.vnet.ibm.com>
Date: Mon, 30 Nov 2015 15:34:27 +0100
Subject: [PATCH 1.5/1.5] S/390: Test that the setmem_long_and pattern is used.

---
 gcc/testsuite/gcc.target/s390/md/setmem_long-1.c | 7 ++++++-
 gcc/testsuite/gcc.target/s390/md/setmem_long-2.c | 6 +++++-
 2 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/gcc/testsuite/gcc.target/s390/md/setmem_long-1.c b/gcc/testsuite/gcc.target/s390/md/setmem_long-1.c
index 9a926ce..c0ed482 100644
--- a/gcc/testsuite/gcc.target/s390/md/setmem_long-1.c
+++ b/gcc/testsuite/gcc.target/s390/md/setmem_long-1.c
@@ -17,4 +17,9 @@ void test2(char *p, int c)
 
 /* Check that the right patterns are used.  */
 /* { dg-final { scan-assembler-times "c:10 .\*\{\\*setmem_long\}" 1 } } */
-/* { dg-final { scan-assembler-times "c:15 .\*\{\\*setmem_long\}" 1 } } */
+/* { dg-final { scan-assembler-times "c:15 .\*\{\\*setmem_long_and\}" 1 } } */
+
+/* Check that the setmem_long_and pattern is used properly.  */
+/* { dg-final { scan-assembler-not "\tsllg\t" } } */
+/* { dg-final { scan-assembler-not "\tllgcr\t" } } */
+/* { dg-final { scan-assembler-not "\tn\t" } } */
diff --git a/gcc/testsuite/gcc.target/s390/md/setmem_long-2.c b/gcc/testsuite/gcc.target/s390/md/setmem_long-2.c
index ce6aa42..8f29071 100644
--- a/gcc/testsuite/gcc.target/s390/md/setmem_long-2.c
+++ b/gcc/testsuite/gcc.target/s390/md/setmem_long-2.c
@@ -17,4 +17,8 @@ void test2(char *p, int c)
 
 /* Check that the right patterns are used.  */
 /* { dg-final { scan-assembler-times "c:10 .\*\{\\*setmem_long_31z\}" 1 } } */
-/* { dg-final { scan-assembler-times "c:15 .\*\{\\*setmem_long_31z\}" 1 } } */
+/* { dg-final { scan-assembler-times "c:15 .\*\{\\*setmem_long_and\}" 1 } } */
+
+/* Check that the setmem_long_and pattern is used properly.  */
+/* { dg-final { scan-assembler-not "\tllcr\t" } } */
+/* { dg-final { scan-assembler-not "\tn\t" } } */
-- 
2.3.0

Re: S/390: Fix warnings in "*setmem_long..." patterns.

[Andreas Krebbel]

On 11/30/2015 04:11 PM, Dominik Vogt wrote:
> The attached patch fixes some warnings generated by the setmem...
> patterns in s390.md during build and add test cases for the
> patterns.  The patch is to be added on to p of the movstr patch:
> https://gcc.gnu.org/ml/gcc-patches/2015-11/msg03485.html
> 
> The test cases validate that the patterns are actually used, but
> at the moment the setmem_long_and pattern is never actually used
> and thus the test case would fail.  So I've split the patch in two
> (both attached to this message) to activate this part of the test
> once we've fixed that.
> 
> The patch has passed the SPEC2006 testsuite without any measurable
> changes in performance.

Shouldn't we instead describe the whole setmem operation as unspec including the other operands as
well? The semantics of the introduced UNSPEC_P_TO_BLK operation is not clear to me.  It suggests to
be some kind of "cast" which it isn't. In fact it is not able to do its job without the length which
is specified as use outside the unspec.

Bye,

-Andreas-

Re: S/390: Fix warnings in "*setmem_long..." patterns.

[Ulrich Weigand]

Andreas Krebbel wrote:
> On 11/30/2015 04:11 PM, Dominik Vogt wrote:
> > The attached patch fixes some warnings generated by the setmem...
> > patterns in s390.md during build and add test cases for the
> > patterns.  The patch is to be added on to p of the movstr patch:
> > https://gcc.gnu.org/ml/gcc-patches/2015-11/msg03485.html
> > 
> > The test cases validate that the patterns are actually used, but
> > at the moment the setmem_long_and pattern is never actually used
> > and thus the test case would fail.  So I've split the patch in two
> > (both attached to this message) to activate this part of the test
> > once we've fixed that.
> > 
> > The patch has passed the SPEC2006 testsuite without any measurable
> > changes in performance.
> 
> Shouldn't we instead describe the whole setmem operation as unspec including the other operands as
> well? The semantics of the introduced UNSPEC_P_TO_BLK operation is not clear to me.  It suggests to
> be some kind of "cast" which it isn't. In fact it is not able to do its job without the length which
> is specified as use outside the unspec.

Well, I guess I suggested to Dominik to leave the basic
[parallel
  (set (dst:BLK) (src:BLK))
  (use (length)]
structure in place; my understanding is that the middle-end recognizes
this as a block move.  As "source" in this case we'd use a BLKmode
operand that consist iof the same byte replicated a number of times.

If we were to use just a single UNSPEC, how would we indicate to the
middle-end that a block of memory is modified, without using too coarse-
grained clobbers?

However, I agree that UNSPEC_P_TO_BLK really should also get the length
as input, to make it have precisely defined semantics.  Also, I'd rather
use a more descriptive name, like UNSPEC_REPLICATE_BYTE or the like.

What would you think about something like the following?

(define_insn "*setmem_long"
  [(clobber (match_operand:<DBL> 0 "register_operand" "=d"))
   (set (mem:BLK (subreg:P (match_operand:<DBL> 3 "register_operand" "0") 0))
        (unspec:BLK [(match_operand:P 2 "shift_count_or_setmem_operand" "Y")
                     (subreg:P (match_dup 3) 1)] UNSPEC_REPLICATE_BYTE))
   (use (match_operand:<DBL> 1 "register_operand" "d"))
   (clobber (reg:CC CC_REGNUM))]

[ Not sure if we'd need an extra (use (match_dup 3)) any more. ]

B.t.w. this is certainly wrong and cannot be generated by common code:
        (and:BLK (unspec:BLK
	      [(match_operand:P 2 "shift_count_or_setmem_operand" "Y")]
	      UNSPEC_P_TO_BLK)
 	     (match_operand 4 "const_int_operand"             "n"))
(This explains why the pattern would never match.)

The AND should be on the filler byte instead:
        (unspec:BLK [(and:P (match_operand:P 2 "shift_count_or_setmem_operand" "Y")
 	                    (match_operand:P 4 "const_int_operand"             "n"))
                     (subreg:P (match_dup 3) 1)] UNSPEC_REPLICATE_BYTE))

Bye,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU/Linux compilers and toolchain
  Ulrich.Weigand@de.ibm.com

Re: S/390: Fix warnings in "*setmem_long..." patterns.

[Andreas Krebbel]

On 11/30/2015 06:11 PM, Ulrich Weigand wrote:
...
> However, I agree that UNSPEC_P_TO_BLK really should also get the length
> as input, to make it have precisely defined semantics.  Also, I'd rather
> use a more descriptive name, like UNSPEC_REPLICATE_BYTE or the like.
> 
> What would you think about something like the following?
> 
> (define_insn "*setmem_long"
>   [(clobber (match_operand:<DBL> 0 "register_operand" "=d"))
>    (set (mem:BLK (subreg:P (match_operand:<DBL> 3 "register_operand" "0") 0))
>         (unspec:BLK [(match_operand:P 2 "shift_count_or_setmem_operand" "Y")
>                      (subreg:P (match_dup 3) 1)] UNSPEC_REPLICATE_BYTE))
>    (use (match_operand:<DBL> 1 "register_operand" "d"))
>    (clobber (reg:CC CC_REGNUM))]

Fine with me. Thanks!

Bye,

-Andreas-

Re: S/390: Fix warnings in "*setmem_long..." patterns.

[Dominik Vogt]

[-- Attachment #1: Type: text/plain, Size: 2670 bytes --]

On Mon, Nov 30, 2015 at 06:11:33PM +0100, Ulrich Weigand wrote:
> On 11/30/2015 04:11 PM, Dominik Vogt wrote:
> > The attached patch fixes some warnings generated by the setmem...
> > patterns in s390.md during build and add test cases for the
> > patterns.  The patch is to be added on to p of the movstr patch:
> > https://gcc.gnu.org/ml/gcc-patches/2015-11/msg03485.html
> > 
> > The test cases validate that the patterns are actually used, but
> > at the moment the setmem_long_and pattern is never actually used
> > and thus the test case would fail.  So I've split the patch in two
> > (both attached to this message) to activate this part of the test
> > once we've fixed that.
> > 
> > The patch has passed the SPEC2006 testsuite without any measurable
> > changes in performance.
> 
> What would you think about something like the following?
> 
> (define_insn "*setmem_long"
>   [(clobber (match_operand:<DBL> 0 "register_operand" "=d"))
>    (set (mem:BLK (subreg:P (match_operand:<DBL> 3 "register_operand" "0") 0))
>         (unspec:BLK [(match_operand:P 2 "shift_count_or_setmem_operand" "Y")
>                      (subreg:P (match_dup 3) 1)] UNSPEC_REPLICATE_BYTE))
>    (use (match_operand:<DBL> 1 "register_operand" "d"))
>    (clobber (reg:CC CC_REGNUM))]

New patch attached (patch 1.5 and ChangeLog are the same).  I've
swapped the operands 1 and 3 so that the numbering is the same as
before.  I think there are still a couple of problems with the
patched code:

1.

The new pattern has "(use (match_operand 3))" where the old one
just had match_dup (which did not express that a register pair was
required).  The expander function now requires a fourth, unused
argument that I don't know how to get rid of.

  emit_insn (gen_setmem_long_di (dst, convert_to_mode (Pmode, len, 1),
                                      val, NULL_RTX));
                                           ^^^^^^^^

2.

I think the pattern should express that the register pair with the
destination address and length gets clobbered by the mvcle
instruction, and I'm not sure whether it's necessary to tell Gcc
explicitly that the register pair with the source address and
legth gets zeroed.

> [ Not sure if we'd need an extra (use (match_dup 3)) any more. ]
> 
> B.t.w. this is certainly wrong and cannot be generated by common code:
>         (and:BLK (unspec:BLK
> 	      [(match_operand:P 2 "shift_count_or_setmem_operand" "Y")]
> 	      UNSPEC_P_TO_BLK)
>  	     (match_operand 4 "const_int_operand"             "n"))
> (This explains why the pattern would never match.)

It never matched before this change either.

Ciao

Dominik ^_^  ^_^

-- 

Dominik Vogt
IBM Germany

[-- Attachment #2: 0001-0001b-v2-Changelog --]
[-- Type: text/plain, Size: 368 bytes --]

gcc/ChangeLog

	* config/s390/s390.c (s390_expand_setmem): Use new expanders.
	* config/s390/s390.md ("*setmem_long")
	("*setmem_long_and", "*setmem_long_31z"): Fix warnings.
	("setmem_long_<P:mode>"): New expanders.
	("setmem_long"): Removed.

gcc/testsuite/ChangeLog

	* gcc.target/s390/md/setmem_long-1.c: New test.
	* gcc.target/s390/md/setmem_long-2.c: New test.

[-- Attachment #3: 0001-v2-S-390-Fix-warnings-in-setmem_long.-patterns.patch --]
[-- Type: text/x-diff, Size: 6714 bytes --]

From 0e1bc4be3466b0f07b1d5c1334e3717802a7db82 Mon Sep 17 00:00:00 2001
From: Dominik Vogt <vogt@linux.vnet.ibm.com>
Date: Wed, 4 Nov 2015 03:16:24 +0100
Subject: [PATCH 1/1.5] S/390: Fix warnings in "*setmem_long..." patterns.

---
 gcc/config/s390/s390.c                           |  7 +++-
 gcc/config/s390/s390.md                          | 51 ++++++++++++++----------
 gcc/testsuite/gcc.target/s390/md/setmem_long-1.c | 20 ++++++++++
 gcc/testsuite/gcc.target/s390/md/setmem_long-2.c | 20 ++++++++++
 4 files changed, 75 insertions(+), 23 deletions(-)
 create mode 100644 gcc/testsuite/gcc.target/s390/md/setmem_long-1.c
 create mode 100644 gcc/testsuite/gcc.target/s390/md/setmem_long-2.c

diff --git a/gcc/config/s390/s390.c b/gcc/config/s390/s390.c
index 40ee2f7..df7af91 100644
--- a/gcc/config/s390/s390.c
+++ b/gcc/config/s390/s390.c
@@ -5178,7 +5178,12 @@ s390_expand_setmem (rtx dst, rtx len, rtx val)
   else if (TARGET_MVCLE)
     {
       val = force_not_mem (convert_modes (Pmode, QImode, val, 1));
-      emit_insn (gen_setmem_long (dst, convert_to_mode (Pmode, len, 1), val));
+      if (TARGET_64BIT)
+	emit_insn (gen_setmem_long_di (dst, convert_to_mode (Pmode, len, 1),
+				       val, NULL_RTX));
+      else
+	emit_insn (gen_setmem_long_si (dst, convert_to_mode (Pmode, len, 1),
+				       val, NULL_RTX));
     }
 
   else
diff --git a/gcc/config/s390/s390.md b/gcc/config/s390/s390.md
index 75e9af7..e093fd3 100644
--- a/gcc/config/s390/s390.md
+++ b/gcc/config/s390/s390.md
@@ -70,6 +70,9 @@
    ; Copy CC as is into the lower 2 bits of an integer register
    UNSPEC_CC_TO_INT
 
+   ; Convert Pmode to BLKmode
+   UNSPEC_REPLICATE_BYTE
+
    ; GOT/PLT and lt-relative accesses
    UNSPEC_LTREL_OFFSET
    UNSPEC_LTREL_BASE
@@ -3281,13 +3284,13 @@
 
 ; Initialize a block of arbitrary length with (operands[2] % 256).
 
-(define_expand "setmem_long"
+(define_expand "setmem_long_<P:mode>"
   [(parallel
-    [(clobber (match_dup 1))
-     (set (match_operand:BLK 0 "memory_operand" "")
-          (match_operand 2 "shift_count_or_setmem_operand" ""))
-     (use (match_operand 1 "general_operand" ""))
-     (use (match_dup 3))
+    [(clobber (match_operand:<DBL> 0 "register_operand" "=d"))
+     (set (mem:BLK (subreg:P (match_operand:<DBL> 1 "register_operand" "0") 0))
+	  (unspec:BLK [(match_operand:P 2 "shift_count_or_setmem_operand" "Y")
+		      (subreg:P (match_dup 1) 8)] UNSPEC_REPLICATE_BYTE))
+     (use (match_operand:<DBL> 3 "register_operand" "d"))
      (clobber (reg:CC CC_REGNUM))])]
   ""
 {
@@ -3310,11 +3313,12 @@
 })
 
 (define_insn "*setmem_long"
-  [(clobber (match_operand:<DBL> 0 "register_operand" "=d"))
-   (set (mem:BLK (subreg:P (match_operand:<DBL> 3 "register_operand" "0") 0))
-        (match_operand 2 "shift_count_or_setmem_operand" "Y"))
-   (use (match_dup 3))
-   (use (match_operand:<DBL> 1 "register_operand" "d"))
+  [(clobber
+    (mem:BLK (subreg:P (match_operand:<DBL> 0 "register_operand" "=d") 0)))
+   (set (mem:BLK (subreg:P (match_operand:<DBL> 1 "register_operand" "0") 0))
+        (unspec:BLK [(match_operand:P 2 "shift_count_or_setmem_operand" "Y")
+                     (subreg:P (match_dup 1) 8)] UNSPEC_REPLICATE_BYTE))
+   (use (match_operand:<DBL> 3 "register_operand" "d"))
    (clobber (reg:CC CC_REGNUM))]
   "TARGET_64BIT || !TARGET_ZARCH"
   "mvcle\t%0,%1,%Y2\;jo\t.-4"
@@ -3322,12 +3326,14 @@
    (set_attr "type" "vs")])
 
 (define_insn "*setmem_long_and"
-  [(clobber (match_operand:<DBL> 0 "register_operand" "=d"))
-   (set (mem:BLK (subreg:P (match_operand:<DBL> 3 "register_operand" "0") 0))
-        (and (match_operand 2 "shift_count_or_setmem_operand" "Y")
-	     (match_operand 4 "const_int_operand"             "n")))
-   (use (match_dup 3))
-   (use (match_operand:<DBL> 1 "register_operand" "d"))
+  [(clobber
+    (mem:BLK (subreg:P (match_operand:<DBL> 0 "register_operand" "=d") 0)))
+   (set (mem:BLK (subreg:P (match_operand:<DBL> 1 "register_operand" "0") 0))
+        (unspec:BLK [(and:P
+		      (match_operand:P 2 "shift_count_or_setmem_operand" "Y")
+		      (match_operand:P 4 "const_int_operand"             "n"))
+		    (subreg:P (match_dup 1) 8)] UNSPEC_REPLICATE_BYTE))
+   (use (match_operand:<DBL> 3 "register_operand" "d"))
    (clobber (reg:CC CC_REGNUM))]
   "(TARGET_64BIT || !TARGET_ZARCH) &&
    (INTVAL (operands[4]) & 255) == 255"
@@ -3336,11 +3342,12 @@
    (set_attr "type" "vs")])
 
 (define_insn "*setmem_long_31z"
-  [(clobber (match_operand:TI 0 "register_operand" "=d"))
-   (set (mem:BLK (subreg:SI (match_operand:TI 3 "register_operand" "0") 4))
-        (match_operand 2 "shift_count_or_setmem_operand" "Y"))
-   (use (match_dup 3))
-   (use (match_operand:TI 1 "register_operand" "d"))
+  [(clobber
+    (mem:BLK (subreg:SI (match_operand:TI 0 "register_operand" "=d") 4)))
+   (set (mem:BLK (subreg:SI (match_operand:TI 1 "register_operand" "0") 0))
+        (unspec:BLK [(match_operand:P 2 "shift_count_or_setmem_operand" "Y")
+                     (subreg:P (match_dup 1) 8)] UNSPEC_REPLICATE_BYTE))
+   (use (match_operand:TI 3 "register_operand" "d"))
    (clobber (reg:CC CC_REGNUM))]
   "!TARGET_64BIT && TARGET_ZARCH"
   "mvcle\t%0,%1,%Y2\;jo\t.-4"
diff --git a/gcc/testsuite/gcc.target/s390/md/setmem_long-1.c b/gcc/testsuite/gcc.target/s390/md/setmem_long-1.c
new file mode 100644
index 0000000..9a926ce
--- /dev/null
+++ b/gcc/testsuite/gcc.target/s390/md/setmem_long-1.c
@@ -0,0 +1,20 @@
+/* Machine description pattern tests.  */
+
+/* { dg-do compile { target { lp64 } } } */
+/* { dg-options "-mmvcle -dP" } */
+
+#include <string.h>
+
+void test(char *p, char c)
+{
+  __builtin_memset (p, c, 10000);
+}
+
+void test2(char *p, int c)
+{
+  __builtin_memset (p, (char)c, 10000);
+}
+
+/* Check that the right patterns are used.  */
+/* { dg-final { scan-assembler-times "c:10 .\*\{\\*setmem_long\}" 1 } } */
+/* { dg-final { scan-assembler-times "c:15 .\*\{\\*setmem_long\}" 1 } } */
diff --git a/gcc/testsuite/gcc.target/s390/md/setmem_long-2.c b/gcc/testsuite/gcc.target/s390/md/setmem_long-2.c
new file mode 100644
index 0000000..ce6aa42
--- /dev/null
+++ b/gcc/testsuite/gcc.target/s390/md/setmem_long-2.c
@@ -0,0 +1,20 @@
+/* Machine description pattern tests.  */
+
+/* { dg-do compile { target { ! lp64 } } } */
+/* { dg-options "-mmvcle -mzarch -dP" } */
+
+#include <string.h>
+
+void test(char *p, char c)
+{
+  __builtin_memset (p, c, 10000);
+}
+
+void test2(char *p, int c)
+{
+  __builtin_memset (p, (char)c, 10000);
+}
+
+/* Check that the right patterns are used.  */
+/* { dg-final { scan-assembler-times "c:10 .\*\{\\*setmem_long_31z\}" 1 } } */
+/* { dg-final { scan-assembler-times "c:15 .\*\{\\*setmem_long_31z\}" 1 } } */
-- 
2.3.0


[-- Attachment #4: 0001b-v2-S-390-Test-that-the-setmem_long_and-pattern-is-used.patch --]
[-- Type: text/x-diff, Size: 1991 bytes --]

From 0b14553b94c2a2b27bfbcdfadd483c57694c9127 Mon Sep 17 00:00:00 2001
From: Dominik Vogt <vogt@linux.vnet.ibm.com>
Date: Mon, 30 Nov 2015 15:34:27 +0100
Subject: [PATCH 1.5/1.5] S/390: Test that the setmem_long_and pattern is used.

---
 gcc/testsuite/gcc.target/s390/md/setmem_long-1.c | 7 ++++++-
 gcc/testsuite/gcc.target/s390/md/setmem_long-2.c | 6 +++++-
 2 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/gcc/testsuite/gcc.target/s390/md/setmem_long-1.c b/gcc/testsuite/gcc.target/s390/md/setmem_long-1.c
index 9a926ce..c0ed482 100644
--- a/gcc/testsuite/gcc.target/s390/md/setmem_long-1.c
+++ b/gcc/testsuite/gcc.target/s390/md/setmem_long-1.c
@@ -17,4 +17,9 @@ void test2(char *p, int c)
 
 /* Check that the right patterns are used.  */
 /* { dg-final { scan-assembler-times "c:10 .\*\{\\*setmem_long\}" 1 } } */
-/* { dg-final { scan-assembler-times "c:15 .\*\{\\*setmem_long\}" 1 } } */
+/* { dg-final { scan-assembler-times "c:15 .\*\{\\*setmem_long_and\}" 1 } } */
+
+/* Check that the setmem_long_and pattern is used properly.  */
+/* { dg-final { scan-assembler-not "\tsllg\t" } } */
+/* { dg-final { scan-assembler-not "\tllgcr\t" } } */
+/* { dg-final { scan-assembler-not "\tn\t" } } */
diff --git a/gcc/testsuite/gcc.target/s390/md/setmem_long-2.c b/gcc/testsuite/gcc.target/s390/md/setmem_long-2.c
index ce6aa42..8f29071 100644
--- a/gcc/testsuite/gcc.target/s390/md/setmem_long-2.c
+++ b/gcc/testsuite/gcc.target/s390/md/setmem_long-2.c
@@ -17,4 +17,8 @@ void test2(char *p, int c)
 
 /* Check that the right patterns are used.  */
 /* { dg-final { scan-assembler-times "c:10 .\*\{\\*setmem_long_31z\}" 1 } } */
-/* { dg-final { scan-assembler-times "c:15 .\*\{\\*setmem_long_31z\}" 1 } } */
+/* { dg-final { scan-assembler-times "c:15 .\*\{\\*setmem_long_and\}" 1 } } */
+
+/* Check that the setmem_long_and pattern is used properly.  */
+/* { dg-final { scan-assembler-not "\tllcr\t" } } */
+/* { dg-final { scan-assembler-not "\tn\t" } } */
-- 
2.3.0

Re: S/390: Fix warnings in "*setmem_long..." patterns.

[Dominik Vogt]

On Tue, Dec 01, 2015 at 10:59:54AM +0100, Dominik Vogt wrote:
> @@ -3336,11 +3342,12 @@
>     (set_attr "type" "vs")])
>  
>  (define_insn "*setmem_long_31z"
> -  [(clobber (match_operand:TI 0 "register_operand" "=d"))
> -   (set (mem:BLK (subreg:SI (match_operand:TI 3 "register_operand" "0") 4))
> -        (match_operand 2 "shift_count_or_setmem_operand" "Y"))
> -   (use (match_dup 3))
> -   (use (match_operand:TI 1 "register_operand" "d"))
> +  [(clobber
> +    (mem:BLK (subreg:SI (match_operand:TI 0 "register_operand" "=d") 4)))
> +   (set (mem:BLK (subreg:SI (match_operand:TI 1 "register_operand" "0") 0))
> +        (unspec:BLK [(match_operand:P 2 "shift_count_or_setmem_operand" "Y")
                         ^^^^^^^^^^^^^^^

match_operand:SI

> +                     (subreg:P (match_dup 1) 8)] UNSPEC_REPLICATE_BYTE))
                         ^^^^^^^^
subreg:SI

> +   (use (match_operand:TI 3 "register_operand" "d"))
>     (clobber (reg:CC CC_REGNUM))]
>    "!TARGET_64BIT && TARGET_ZARCH"
>    "mvcle\t%0,%1,%Y2\;jo\t.-4"

Ciao

Dominik ^_^  ^_^

-- 

Dominik Vogt
IBM Germany

Re: S/390: Fix warnings in "*setmem_long..." patterns.

[Dominik Vogt]

[-- Attachment #1: Type: text/plain, Size: 317 bytes --]

Hopefully, this is correct now; it does pass the functional test case
that's part of the patch.  Unfortunately the define_insn patters
had to be duplicated because of the new subreg offsets.  Not sure
whether I've missed any "use" patterns that should be added.

Ciao

Dominik ^_^  ^_^

-- 

Dominik Vogt
IBM Germany

[-- Attachment #2: 0001-v3-ChangeLog --]
[-- Type: text/plain, Size: 456 bytes --]

gcc/ChangeLog

	* config/s390/s390.c (s390_expand_setmem): Use new expanders.
	* config/s390/s390.md ("*setmem_long")
	("*setmem_long_and", "*setmem_long_31z"): Fix warnings.
	("*setmem_long_and_64", "*setmem_long_and_31", "*setmem_long_and_31z")
	("*setmem_long_64", "*setmem_long_31"): Renamed and duplicated.
	("setmem_long_<P:mode>"): New expanders.
	("setmem_long"): Removed.

gcc/testsuite/ChangeLog

	* gcc.target/s390/md/setmem_long-1.c: New test.

[-- Attachment #3: 0001-v3-S-390-Fix-warnings-in-setmem_long.-patterns.patch --]
[-- Type: text/x-diff, Size: 8309 bytes --]

From 922d200afbe8493e62b0ffb300fbac11356469c8 Mon Sep 17 00:00:00 2001
From: Dominik Vogt <vogt@linux.vnet.ibm.com>
Date: Wed, 4 Nov 2015 03:16:24 +0100
Subject: [PATCH 1/1.5] S/390: Fix warnings in "*setmem_long..." patterns.

---
 gcc/config/s390/s390.c                           |  7 +-
 gcc/config/s390/s390.md                          | 89 ++++++++++++++++++------
 gcc/testsuite/gcc.target/s390/md/setmem_long-1.c | 64 +++++++++++++++++
 3 files changed, 138 insertions(+), 22 deletions(-)
 create mode 100644 gcc/testsuite/gcc.target/s390/md/setmem_long-1.c

diff --git a/gcc/config/s390/s390.c b/gcc/config/s390/s390.c
index 7e7ed45..1a77437 100644
--- a/gcc/config/s390/s390.c
+++ b/gcc/config/s390/s390.c
@@ -5203,7 +5203,12 @@ s390_expand_setmem (rtx dst, rtx len, rtx val)
   else if (TARGET_MVCLE)
     {
       val = force_not_mem (convert_modes (Pmode, QImode, val, 1));
-      emit_insn (gen_setmem_long (dst, convert_to_mode (Pmode, len, 1), val));
+      if (TARGET_64BIT)
+	emit_insn (gen_setmem_long_di (dst, convert_to_mode (Pmode, len, 1),
+				       val));
+      else
+	emit_insn (gen_setmem_long_si (dst, convert_to_mode (Pmode, len, 1),
+				       val));
     }
 
   else
diff --git a/gcc/config/s390/s390.md b/gcc/config/s390/s390.md
index 7eca315..27e5c7f 100644
--- a/gcc/config/s390/s390.md
+++ b/gcc/config/s390/s390.md
@@ -70,6 +70,9 @@
    ; Copy CC as is into the lower 2 bits of an integer register
    UNSPEC_CC_TO_INT
 
+   ; Convert Pmode to BLKmode
+   UNSPEC_REPLICATE_BYTE
+
    ; GOT/PLT and lt-relative accesses
    UNSPEC_LTREL_OFFSET
    UNSPEC_LTREL_BASE
@@ -3281,12 +3284,12 @@
 
 ; Initialize a block of arbitrary length with (operands[2] % 256).
 
-(define_expand "setmem_long"
+(define_expand "setmem_long_<P:mode>"
   [(parallel
     [(clobber (match_dup 1))
      (set (match_operand:BLK 0 "memory_operand" "")
-          (match_operand 2 "shift_count_or_setmem_operand" ""))
-     (use (match_operand 1 "general_operand" ""))
+	  (unspec:BLK [(match_operand:P 2 "shift_count_or_setmem_operand" "Y")
+		      (match_dup 4)] UNSPEC_REPLICATE_BYTE))
      (use (match_dup 3))
      (clobber (reg:CC CC_REGNUM))])]
   ""
@@ -3307,30 +3310,29 @@
   operands[0] = replace_equiv_address_nv (operands[0], addr0);
   operands[1] = reg0;
   operands[3] = reg1;
+  operands[4] = gen_lowpart (Pmode, operands[1]);
 })
 
-(define_insn "*setmem_long"
-  [(clobber (match_operand:<DBL> 0 "register_operand" "=d"))
-   (set (mem:BLK (subreg:P (match_operand:<DBL> 3 "register_operand" "0") 0))
-        (match_operand 2 "shift_count_or_setmem_operand" "Y"))
-   (use (match_dup 3))
-   (use (match_operand:<DBL> 1 "register_operand" "d"))
+(define_insn "*setmem_long_64"
+  [(clobber (match_operand:TI 0 "register_operand" "=d"))
+   (set (mem:BLK (subreg:DI (match_operand:TI 3 "register_operand" "0") 0))
+        (unspec:BLK [(match_operand:DI 2 "shift_count_or_setmem_operand" "Y")
+		     (subreg:DI (match_dup 3) 8)] UNSPEC_REPLICATE_BYTE))
+   (use (match_operand:TI 1 "register_operand" "d"))
    (clobber (reg:CC CC_REGNUM))]
-  "TARGET_64BIT || !TARGET_ZARCH"
+  "TARGET_64BIT"
   "mvcle\t%0,%1,%Y2\;jo\t.-4"
   [(set_attr "length" "8")
    (set_attr "type" "vs")])
 
-(define_insn "*setmem_long_and"
-  [(clobber (match_operand:<DBL> 0 "register_operand" "=d"))
-   (set (mem:BLK (subreg:P (match_operand:<DBL> 3 "register_operand" "0") 0))
-        (and (match_operand 2 "shift_count_or_setmem_operand" "Y")
-	     (match_operand 4 "const_int_operand"             "n")))
-   (use (match_dup 3))
-   (use (match_operand:<DBL> 1 "register_operand" "d"))
+(define_insn "*setmem_long_31"
+  [(clobber (match_operand:DI 0 "register_operand" "=d"))
+   (set (mem:BLK (subreg:SI (match_operand:DI 3 "register_operand" "0") 0))
+        (unspec:BLK [(match_operand:SI 2 "shift_count_or_setmem_operand" "Y")
+		     (subreg:SI (match_dup 3) 4)] UNSPEC_REPLICATE_BYTE))
+   (use (match_operand:DI 1 "register_operand" "d"))
    (clobber (reg:CC CC_REGNUM))]
-  "(TARGET_64BIT || !TARGET_ZARCH) &&
-   (INTVAL (operands[4]) & 255) == 255"
+  "!TARGET_64BIT && !TARGET_ZARCH"
   "mvcle\t%0,%1,%Y2\;jo\t.-4"
   [(set_attr "length" "8")
    (set_attr "type" "vs")])
@@ -3338,8 +3340,8 @@
 (define_insn "*setmem_long_31z"
   [(clobber (match_operand:TI 0 "register_operand" "=d"))
    (set (mem:BLK (subreg:SI (match_operand:TI 3 "register_operand" "0") 4))
-        (match_operand 2 "shift_count_or_setmem_operand" "Y"))
-   (use (match_dup 3))
+        (unspec:BLK [(match_operand:SI 2 "shift_count_or_setmem_operand" "Y")
+		     (subreg:SI (match_dup 3) 12)] UNSPEC_REPLICATE_BYTE))
    (use (match_operand:TI 1 "register_operand" "d"))
    (clobber (reg:CC CC_REGNUM))]
   "!TARGET_64BIT && TARGET_ZARCH"
@@ -3347,6 +3349,51 @@
   [(set_attr "length" "8")
    (set_attr "type" "vs")])
 
+(define_insn "*setmem_long_and_64"
+  [(clobber (match_operand:TI 0 "register_operand" "=d"))
+   (set (mem:BLK (subreg:DI (match_operand:TI 3 "register_operand" "0") 0))
+        (unspec:BLK [(and:DI
+		      (match_operand:DI 2 "shift_count_or_setmem_operand" "Y")
+		      (match_operand:DI 4 "const_int_operand"             "n"))
+		    (subreg:DI (match_dup 3) 8)] UNSPEC_REPLICATE_BYTE))
+   (use (match_operand:TI 1 "register_operand" "d"))
+   (clobber (reg:CC CC_REGNUM))]
+  "TARGET_64BIT &&
+   (INTVAL (operands[4]) & 255) == 255"
+  "mvcle\t%0,%1,%Y2\;jo\t.-4"
+  [(set_attr "length" "8")
+   (set_attr "type" "vs")])
+
+(define_insn "*setmem_long_and_31"
+  [(clobber (match_operand:DI 0 "register_operand" "=d"))
+   (set (mem:BLK (subreg:SI (match_operand:DI 3 "register_operand" "0") 0))
+        (unspec:BLK [(and:SI
+		      (match_operand:SI 2 "shift_count_or_setmem_operand" "Y")
+		      (match_operand:SI 4 "const_int_operand"             "n"))
+		    (subreg:SI (match_dup 3) 4)] UNSPEC_REPLICATE_BYTE))
+   (use (match_operand:DI 1 "register_operand" "d"))
+   (clobber (reg:CC CC_REGNUM))]
+  "(!TARGET_64BIT && !TARGET_ZARCH) &&
+   (INTVAL (operands[4]) & 255) == 255"
+  "mvcle\t%0,%1,%Y2\;jo\t.-4"
+  [(set_attr "length" "8")
+   (set_attr "type" "vs")])
+
+(define_insn "*setmem_long_and_31z"
+  [(clobber (match_operand:TI 0 "register_operand" "=d"))
+   (set (mem:BLK (subreg:SI (match_operand:TI 3 "register_operand" "0") 4))
+        (unspec:BLK [(and:SI
+		      (match_operand:SI 2 "shift_count_or_setmem_operand" "Y")
+		      (match_operand:SI 4 "const_int_operand"             "n"))
+		    (subreg:SI (match_dup 3) 12)] UNSPEC_REPLICATE_BYTE))
+   (use (match_operand:TI 1 "register_operand" "d"))
+   (clobber (reg:CC CC_REGNUM))]
+  "(!TARGET_64BIT && TARGET_ZARCH) &&
+   (INTVAL (operands[4]) & 255) == 255"
+  "mvcle\t%0,%1,%Y2\;jo\t.-4"
+  [(set_attr "length" "8")
+   (set_attr "type" "vs")])
+
 ;
 ; cmpmemM instruction pattern(s).
 ;
diff --git a/gcc/testsuite/gcc.target/s390/md/setmem_long-1.c b/gcc/testsuite/gcc.target/s390/md/setmem_long-1.c
new file mode 100644
index 0000000..dec46ce
--- /dev/null
+++ b/gcc/testsuite/gcc.target/s390/md/setmem_long-1.c
@@ -0,0 +1,64 @@
+/* Machine description pattern tests.  */
+
+/* { dg-do run } */
+/* { dg-options "-mmvcle -dP" } */
+
+#include <string.h>
+#include <stdio.h>
+#include <stdlib.h>
+
+void test(char *p, char c, int len)
+{
+  __builtin_memset(p, c, len);
+}
+
+void test2(char *p, int c, int len)
+{
+  __builtin_memset(p, (char)c, len);
+}
+
+/* Check that the right patterns are used.  */
+/* { dg-final { scan-assembler-times {c:12 .*{[*]setmem_long_[36][14]z?}} 1 } } */
+/* { dg-final { scan-assembler-times {c:17 .*{[*]setmem_long_[36][14]z?}} 1 } } */
+
+#define LEN 500
+char buf[LEN + 2];
+
+void init_buf(void)
+{
+  int i;
+
+  buf[0] = 0;
+  for (i = 1; i <= LEN; i++)
+    buf[i] = (0x10 + (i & 0x3f));
+  buf[LEN + 1] = 0x7f;
+}
+
+void validate_buf(char val, const char *test)
+{
+  int i;
+
+  if (buf[0] != 0)
+    goto error;
+  for (i = 1; i <= LEN; i++)
+    if (buf[i] != val)
+      goto error;
+  if (buf[LEN + 1] != 0x7f)
+    goto error;
+  return;
+
+ error:
+  fprintf(stderr, "error: %s() failed at byte %d (0x%02x)\n", test, i,
+	  (int) buf[i]);
+  exit(1);
+}
+
+int main(void)
+{
+  init_buf();
+  test(buf + 1, 55, LEN);
+  validate_buf(55, "test");
+  init_buf();
+  test(buf + 1, 66, LEN);
+  validate_buf(66, "test2");
+}
-- 
2.3.0


[-- Attachment #4: 0001b-v3-S-390-Make-sure-the-setmem_long_and_patterns-are-use.patch --]
[-- Type: text/x-diff, Size: 1296 bytes --]

From 590e4da997ee294fe4d53e933941cf506691d9f4 Mon Sep 17 00:00:00 2001
From: Dominik Vogt <vogt@linux.vnet.ibm.com>
Date: Wed, 2 Dec 2015 10:30:05 +0100
Subject: [PATCH 1.5/1.5] S/390: Make sure the setmem_long_and_patterns are used.

---
 gcc/testsuite/gcc.target/s390/md/setmem_long-1.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/gcc/testsuite/gcc.target/s390/md/setmem_long-1.c b/gcc/testsuite/gcc.target/s390/md/setmem_long-1.c
index dec46ce..0a4a82c 100644
--- a/gcc/testsuite/gcc.target/s390/md/setmem_long-1.c
+++ b/gcc/testsuite/gcc.target/s390/md/setmem_long-1.c
@@ -19,7 +19,13 @@ void test2(char *p, int c, int len)
 
 /* Check that the right patterns are used.  */
 /* { dg-final { scan-assembler-times {c:12 .*{[*]setmem_long_[36][14]z?}} 1 } } */
-/* { dg-final { scan-assembler-times {c:17 .*{[*]setmem_long_[36][14]z?}} 1 } } */
+/* { dg-final { scan-assembler-times {c:17 .*{[*]setmem_long_and_[36][14]z?}} 1 } } */
+
+/* Check that the setmem_long_and pattern is used properly.  */
+/* { dg-final { scan-assembler-not "\tsllg\t" } } */
+/* { dg-final { scan-assembler-not "\tllgcr\t" } } */
+/* { dg-final { scan-assembler-not "\tn\t" } } */
+/* { dg-final { scan-assembler-not "\tllcr\t" } } */
 
 #define LEN 500
 char buf[LEN + 2];
-- 
2.3.0

Re: S/390: Fix warnings in "*setmem_long..." patterns.

[Andreas Krebbel]

On 12/02/2015 11:12 AM, Dominik Vogt wrote:
> Hopefully, this is correct now; it does pass the functional test case
> that's part of the patch.  Unfortunately the define_insn patters
> had to be duplicated because of the new subreg offsets.  

The number of patterns could possibly be reduced using the define_subst machinery.  I'm looking into
this for some other changes. No need to do this right now. We can do this later on-top.

> Not sure
> whether I've missed any "use" patterns that should be added.

With adding the length operand explicitly to the unspec we should have all the uses of the register
pair covered. To my understanding it is correct to remove these as done with your patch.

+   ; Convert Pmode to BLKmode
+   UNSPEC_REPLICATE_BYTE

The comment does not match.

+++ b/gcc/testsuite/gcc.target/s390/md/setmem_long-1.c
@@ -0,0 +1,64 @@
+/* Machine description pattern tests.  */
+
+/* { dg-do run } */
+/* { dg-options "-mmvcle -dP" } */
...
+/* Check that the right patterns are used.  */
+/* { dg-final { scan-assembler-times {c:12 .*{[*]setmem_long_[36][14]z?}} 1 } } */
+/* { dg-final { scan-assembler-times {c:17 .*{[*]setmem_long_[36][14]z?}} 1 } } */

Don't you need a --save-temps as part of the options?

Apart from these things the patch looks good to me now. I'll wait two days for other comments before
applying it (I can do the remaining changes while doing the commit.).

Thanks!

-Andreas-


> 
> Ciao
> 
> Dominik ^_^  ^_^
> 

Re: S/390: Fix warnings in "*setmem_long..." patterns.

[Ulrich Weigand]

Andreas Krebbel wrote:
> On 12/02/2015 11:12 AM, Dominik Vogt wrote:
> > Hopefully, this is correct now; it does pass the functional test case
> > that's part of the patch.  Unfortunately the define_insn patters
> > had to be duplicated because of the new subreg offsets.  
> 
> The number of patterns could possibly be reduced using the define_subst machinery.  I'm looking into
> this for some other changes. No need to do this right now. We can do this later on-top.

For this particular issue, shouldn't a simple mode_attr be OK?
I see that the sh port uses this:

(define_mode_attr lowpart_be [(QI "3") (HI "2")])

  [(set (reg:SI T_REG)
        (eq:SI
          (subreg:QIHI
            (and:SI (match_operand:SI 0 "arith_reg_operand")
                    (match_operand:SI 1 "arith_reg_operand")) <lowpart_be>)
          (const_int 0)))]

Bye,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU/Linux compilers and toolchain
  Ulrich.Weigand@de.ibm.com

Re: S/390: Fix warnings in "*setmem_long..." patterns.

[Andreas Krebbel]

On 12/02/2015 01:51 PM, Ulrich Weigand wrote:
> Andreas Krebbel wrote:
>> On 12/02/2015 11:12 AM, Dominik Vogt wrote:
>>> Hopefully, this is correct now; it does pass the functional test case
>>> that's part of the patch.  Unfortunately the define_insn patters
>>> had to be duplicated because of the new subreg offsets.  
>>
>> The number of patterns could possibly be reduced using the define_subst machinery.  I'm looking into
>> this for some other changes. No need to do this right now. We can do this later on-top.
> 
> For this particular issue, shouldn't a simple mode_attr be OK?
> I see that the sh port uses this:
> 
> (define_mode_attr lowpart_be [(QI "3") (HI "2")])
> 
>   [(set (reg:SI T_REG)
>         (eq:SI
>           (subreg:QIHI
>             (and:SI (match_operand:SI 0 "arith_reg_operand")
>                     (match_operand:SI 1 "arith_reg_operand")) <lowpart_be>)
>           (const_int 0)))]

Unfortunately in our case the attribute value doesn't only depend on the mode.  It also depends on
zarch/esa.  We would need some kind of conditional attribute.

-Andreas-

Re: S/390: Fix warnings in "*setmem_long..." patterns.

[Andreas Krebbel]

On 12/02/2015 02:11 PM, Andreas Krebbel wrote:
> On 12/02/2015 01:51 PM, Ulrich Weigand wrote:
>> Andreas Krebbel wrote:
>>> On 12/02/2015 11:12 AM, Dominik Vogt wrote:
>>>> Hopefully, this is correct now; it does pass the functional test case
>>>> that's part of the patch.  Unfortunately the define_insn patters
>>>> had to be duplicated because of the new subreg offsets.  
>>>
>>> The number of patterns could possibly be reduced using the define_subst machinery.  I'm looking into
>>> this for some other changes. No need to do this right now. We can do this later on-top.
>>
>> For this particular issue, shouldn't a simple mode_attr be OK?
>> I see that the sh port uses this:
>>
>> (define_mode_attr lowpart_be [(QI "3") (HI "2")])
>>
>>   [(set (reg:SI T_REG)
>>         (eq:SI
>>           (subreg:QIHI
>>             (and:SI (match_operand:SI 0 "arith_reg_operand")
>>                     (match_operand:SI 1 "arith_reg_operand")) <lowpart_be>)
>>           (const_int 0)))]
> 
> Unfortunately in our case the attribute value doesn't only depend on the mode.  It also depends on
> zarch/esa.  We would need some kind of conditional attribute.

However, we could probably at least merge the two zarch variants.

The define_subst is probably the right thing for adding the AND around the padding byte operand.

Bye,

-Andreas-

Re: S/390: Fix warnings in "*setmem_long..." patterns.

[Dominik Vogt]

[-- Attachment #1: Type: text/plain, Size: 197 bytes --]

Version 5 with the latest requested changes.  Seems to work now.
I've dropped the extra patch and rather marked the failing tests
as "xfail".

Ciao

Dominik ^_^  ^_^

-- 

Dominik Vogt
IBM Germany

[-- Attachment #2: 0001-v5-ChangeLog --]
[-- Type: text/plain, Size: 371 bytes --]

gcc/ChangeLog

	* config/s390/s390.c (s390_expand_setmem): Use new expanders.
	* config/s390/s390.md ("*setmem_long")
	("*setmem_long_and", "*setmem_long_31z"): Fix warnings.
	("*setmem_long_and_31z"): New define_insn.
	("setmem_long_<P:mode>"): New expanders.
	* (<modesize>): New mode attribute
gcc/testsuite/ChangeLog

	* gcc.target/s390/md/setmem_long-1.c: New test.

[-- Attachment #3: 0001-v5-S-390-Fix-warnings-in-setmem_long.-patterns.patch --]
[-- Type: text/x-diff, Size: 6899 bytes --]

From 74780dc2756ed1c2052f0d6b836799dcad1217e7 Mon Sep 17 00:00:00 2001
From: Dominik Vogt <vogt@linux.vnet.ibm.com>
Date: Wed, 4 Nov 2015 03:16:24 +0100
Subject: [PATCH] S/390: Fix warnings in "*setmem_long..." patterns.

---
 gcc/config/s390/s390.c                           |  7 ++-
 gcc/config/s390/s390.md                          | 50 ++++++++++++++++-----
 gcc/testsuite/gcc.target/s390/md/setmem_long-1.c | 56 ++++++++++++++++++++++++
 3 files changed, 102 insertions(+), 11 deletions(-)
 create mode 100644 gcc/testsuite/gcc.target/s390/md/setmem_long-1.c

diff --git a/gcc/config/s390/s390.c b/gcc/config/s390/s390.c
index 7e7ed45..1a77437 100644
--- a/gcc/config/s390/s390.c
+++ b/gcc/config/s390/s390.c
@@ -5203,7 +5203,12 @@ s390_expand_setmem (rtx dst, rtx len, rtx val)
   else if (TARGET_MVCLE)
     {
       val = force_not_mem (convert_modes (Pmode, QImode, val, 1));
-      emit_insn (gen_setmem_long (dst, convert_to_mode (Pmode, len, 1), val));
+      if (TARGET_64BIT)
+	emit_insn (gen_setmem_long_di (dst, convert_to_mode (Pmode, len, 1),
+				       val));
+      else
+	emit_insn (gen_setmem_long_si (dst, convert_to_mode (Pmode, len, 1),
+				       val));
     }
 
   else
diff --git a/gcc/config/s390/s390.md b/gcc/config/s390/s390.md
index bc24a36..ef1ec92 100644
--- a/gcc/config/s390/s390.md
+++ b/gcc/config/s390/s390.md
@@ -70,6 +70,9 @@
    ; Copy CC as is into the lower 2 bits of an integer register
    UNSPEC_CC_TO_INT
 
+   ; Convert Pmode to BLKmode
+   UNSPEC_REPLICATE_BYTE
+
    ; GOT/PLT and lt-relative accesses
    UNSPEC_LTREL_OFFSET
    UNSPEC_LTREL_BASE
@@ -727,6 +730,9 @@
 ;; In place of GET_MODE_BITSIZE (<MODE>mode)
 (define_mode_attr bitsize [(DI "64") (SI "32") (HI "16") (QI "8")])
 
+;; In place of GET_MODE_SIZE (<MODE>mode)
+(define_mode_attr modesize [(DI "8") (SI "4")])
+
 ;; Allow return and simple_return to be defined from a single template.
 (define_code_iterator ANY_RETURN [return simple_return])
 
@@ -3280,12 +3286,12 @@
 
 ; Initialize a block of arbitrary length with (operands[2] % 256).
 
-(define_expand "setmem_long"
+(define_expand "setmem_long_<P:mode>"
   [(parallel
     [(clobber (match_dup 1))
      (set (match_operand:BLK 0 "memory_operand" "")
-          (match_operand 2 "shift_count_or_setmem_operand" ""))
-     (use (match_operand 1 "general_operand" ""))
+	  (unspec:BLK [(match_operand:P 2 "shift_count_or_setmem_operand" "Y")
+		      (match_dup 4)] UNSPEC_REPLICATE_BYTE))
      (use (match_dup 3))
      (clobber (reg:CC CC_REGNUM))])]
   ""
@@ -3306,13 +3312,17 @@
   operands[0] = replace_equiv_address_nv (operands[0], addr0);
   operands[1] = reg0;
   operands[3] = reg1;
+  operands[4] = gen_lowpart (Pmode, operands[1]);
 })
 
+; Patterns for 31 bit + Esa and 64 bit + Zarch.
+
 (define_insn "*setmem_long"
   [(clobber (match_operand:<DBL> 0 "register_operand" "=d"))
    (set (mem:BLK (subreg:P (match_operand:<DBL> 3 "register_operand" "0") 0))
-        (match_operand 2 "shift_count_or_setmem_operand" "Y"))
-   (use (match_dup 3))
+        (unspec:BLK [(match_operand:P 2 "shift_count_or_setmem_operand" "Y")
+		     (subreg:P (match_dup 3) <modesize>)]
+		     UNSPEC_REPLICATE_BYTE))
    (use (match_operand:<DBL> 1 "register_operand" "d"))
    (clobber (reg:CC CC_REGNUM))]
   "TARGET_64BIT || !TARGET_ZARCH"
@@ -3323,9 +3333,11 @@
 (define_insn "*setmem_long_and"
   [(clobber (match_operand:<DBL> 0 "register_operand" "=d"))
    (set (mem:BLK (subreg:P (match_operand:<DBL> 3 "register_operand" "0") 0))
-        (and (match_operand 2 "shift_count_or_setmem_operand" "Y")
-	     (match_operand 4 "const_int_operand"             "n")))
-   (use (match_dup 3))
+        (unspec:BLK [(and:P
+		      (match_operand:P 2 "shift_count_or_setmem_operand" "Y")
+		      (match_operand:P 4 "const_int_operand"             "n"))
+		    (subreg:P (match_dup 3) <modesize>)]
+		    UNSPEC_REPLICATE_BYTE))
    (use (match_operand:<DBL> 1 "register_operand" "d"))
    (clobber (reg:CC CC_REGNUM))]
   "(TARGET_64BIT || !TARGET_ZARCH) &&
@@ -3334,11 +3346,14 @@
   [(set_attr "length" "8")
    (set_attr "type" "vs")])
 
+; Variants for 31 bit + Zarch, necessary because of the odd in-register offsets
+; of the SImode subregs.
+
 (define_insn "*setmem_long_31z"
   [(clobber (match_operand:TI 0 "register_operand" "=d"))
    (set (mem:BLK (subreg:SI (match_operand:TI 3 "register_operand" "0") 4))
-        (match_operand 2 "shift_count_or_setmem_operand" "Y"))
-   (use (match_dup 3))
+        (unspec:BLK [(match_operand:SI 2 "shift_count_or_setmem_operand" "Y")
+		     (subreg:SI (match_dup 3) 12)] UNSPEC_REPLICATE_BYTE))
    (use (match_operand:TI 1 "register_operand" "d"))
    (clobber (reg:CC CC_REGNUM))]
   "!TARGET_64BIT && TARGET_ZARCH"
@@ -3346,6 +3361,21 @@
   [(set_attr "length" "8")
    (set_attr "type" "vs")])
 
+(define_insn "*setmem_long_and_31z"
+  [(clobber (match_operand:TI 0 "register_operand" "=d"))
+   (set (mem:BLK (subreg:SI (match_operand:TI 3 "register_operand" "0") 4))
+        (unspec:BLK [(and:SI
+		      (match_operand:SI 2 "shift_count_or_setmem_operand" "Y")
+		      (match_operand:SI 4 "const_int_operand"             "n"))
+		    (subreg:SI (match_dup 3) 12)] UNSPEC_REPLICATE_BYTE))
+   (use (match_operand:TI 1 "register_operand" "d"))
+   (clobber (reg:CC CC_REGNUM))]
+  "(!TARGET_64BIT && TARGET_ZARCH) &&
+   (INTVAL (operands[4]) & 255) == 255"
+  "mvcle\t%0,%1,%Y2\;jo\t.-4"
+  [(set_attr "length" "8")
+   (set_attr "type" "vs")])
+
 ;
 ; cmpmemM instruction pattern(s).
 ;
diff --git a/gcc/testsuite/gcc.target/s390/md/setmem_long-1.c b/gcc/testsuite/gcc.target/s390/md/setmem_long-1.c
new file mode 100644
index 0000000..933a698
--- /dev/null
+++ b/gcc/testsuite/gcc.target/s390/md/setmem_long-1.c
@@ -0,0 +1,56 @@
+/* Machine description pattern tests.  */
+
+/* { dg-do run } */
+/* { dg-options "-mmvcle -dP -save-temps" } */
+
+__attribute__ ((noinline))
+void test(char *p, char c, int len)
+{
+  __builtin_memset(p, c, len);
+}
+
+__attribute__ ((noinline))
+void test2(char *p, int c, int len)
+{
+  __builtin_memset(p, (char)c, len);
+}
+
+/* Check that the right patterns are used.  */
+/* { dg-final { scan-assembler-times {c:9 .*{[*]setmem_long_?3?1?z?}} 1 } } */
+/* { dg-final { scan-assembler-times {c:15 .*{[*]setmem_long_and_?3?1?z?}} 1 { xfail *-*-* } } } */
+
+#define LEN 500
+char buf[LEN + 2];
+
+void init_buf(void)
+{
+  int i;
+
+  buf[0] = 0;
+  for (i = 1; i <= LEN; i++)
+    buf[i] = (0x10 + (i & 0x3f));
+  buf[LEN + 1] = 0x7f;
+}
+
+void validate_buf(char val)
+{
+  int i;
+
+  if (buf[0] != 0)
+    __builtin_abort();
+  for (i = 1; i <= LEN; i++)
+    if (buf[i] != val)
+      __builtin_abort();
+  if (buf[LEN + 1] != 0x7f)
+    __builtin_abort();
+}
+
+int main(void)
+{
+  init_buf();
+  test(buf + 1, 55, LEN);
+  validate_buf(55);
+  init_buf();
+  test(buf + 1, 66, LEN);
+  validate_buf(66);
+}
-- 
2.3.0

Re: S/390: Fix warnings in "*setmem_long..." patterns.

[Andreas Krebbel]

On 12/04/2015 06:15 PM, Dominik Vogt wrote:
> Version 5 with the latest requested changes.  Seems to work now.
> I've dropped the extra patch and rather marked the failing tests
> as "xfail".
> 
> Ciao
> 
> Dominik ^_^  ^_^
> 

Patch applied with minor changes:

> +   ; Convert Pmode to BLKmode
> +   UNSPEC_REPLICATE_BYTE

That comment did not really fit after changing the name of the unspec.

> -(define_expand "setmem_long"
> +(define_expand "setmem_long_<P:mode>"
>    [(parallel
>      [(clobber (match_dup 1))
>       (set (match_operand:BLK 0 "memory_operand" "")
> -          (match_operand 2 "shift_count_or_setmem_operand" ""))
> -     (use (match_operand 1 "general_operand" ""))
> +	  (unspec:BLK [(match_operand:P 2 "shift_count_or_setmem_operand" "Y")

Superfluous constraint removed.

Thanks!

-Andreas-