[PATCH] S/390: Wide int support.

[PATCH] S/390: Wide int support.

[Dominik Vogt]

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

The attached patch introduces wide int support to S/390 in order
to resolve a test case failure in gcc.dg/pr68129_1.c that is
caused by an assertion in
simplify-rtx.c:simplify_const_binary_operation().

Ciao

Dominik ^_^  ^_^

-- 

Dominik Vogt
IBM Germany

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

gcc/ChangeLog

	* config/s390/s390.c (s390_rtx_costs)
	(s390_cannot_force_const_mem, legitimate_pic_operand_p)
	(s390_preferred_reload_class, s390_reload_symref_address)
	(legitimate_reload_constant_p, print_operand): Wide int support.
	* config/s390/predicates.md ("const0_operand", "constm1_operand")
	("consttable_operand"): Likewise.
	("larl_operand"): Add a comment.
	* config/s390/s390.h (TARGET_SUPPORTS_WIDE_INT): Enable wide int
	support.

[-- Attachment #3: 0001-S-390-Wide-int-support.patch --]
[-- Type: text/x-diff, Size: 6279 bytes --]

From a2909e33789375d0217957defd335491d341918b Mon Sep 17 00:00:00 2001
From: Dominik Vogt <vogt@linux.vnet.ibm.com>
Date: Thu, 10 Dec 2015 11:51:08 +0100
Subject: [PATCH] S/390: Wide int support.

This fixes the assertion in simplify-rtx.c:simplify_const_binary_operation()
triggered by gcc.dg/pr68129_1.c.
---
 gcc/config/s390/predicates.md |  9 ++++++---
 gcc/config/s390/s390.c        | 26 ++++++++++++++++----------
 gcc/config/s390/s390.h        |  2 ++
 3 files changed, 24 insertions(+), 13 deletions(-)

diff --git a/gcc/config/s390/predicates.md b/gcc/config/s390/predicates.md
index 6a5ebbb..75cecf0 100644
--- a/gcc/config/s390/predicates.md
+++ b/gcc/config/s390/predicates.md
@@ -26,12 +26,12 @@
 
 ;; Return true if OP a const 0 operand (int/float/vector).
 (define_predicate "const0_operand"
-  (and (match_code "const_int,const_double,const_vector")
+  (and (match_code "const_int,const_wide_int,const_double,const_vector")
        (match_test "op == CONST0_RTX (mode)")))
 
 ;; Return true if OP an all ones operand (int/float/vector).
 (define_predicate "constm1_operand"
-  (and (match_code "const_int, const_double,const_vector")
+  (and (match_code "const_int,const_wide_int,const_double,const_vector")
        (match_test "op == CONSTM1_RTX (mode)")))
 
 ;; Return true if OP is a 4 bit mask operand
@@ -42,7 +42,7 @@
 ;; Return true if OP is constant.
 
 (define_special_predicate "consttable_operand"
-  (and (match_code "symbol_ref, label_ref, const, const_int, const_double, const_vector")
+  (and (match_code "symbol_ref, label_ref, const, const_int, const_wide_int, const_double, const_vector")
        (match_test "CONSTANT_P (op)")))
 
 ;; Return true if OP is a valid S-type operand.
@@ -121,6 +121,9 @@
 ;;  Return true if OP a valid operand for the LARL instruction.
 
 (define_predicate "larl_operand"
+; Note: Although CONST_INT and CONST_DOUBLE are not handled in this predicate,
+; at least one of them needs to appear or otherwise safe_predicate_mode will
+; assume that a DImode LABEL_REF is not accepted either (see genrecog.c).
   (match_code "label_ref, symbol_ref, const, const_int, const_double")
 {
   /* Allow labels and local symbols.  */
diff --git a/gcc/config/s390/s390.c b/gcc/config/s390/s390.c
index f8928b9..bed58d8 100644
--- a/gcc/config/s390/s390.c
+++ b/gcc/config/s390/s390.c
@@ -1323,7 +1323,7 @@ s390_tm_ccmode (rtx op1, rtx op2, bool mixed)
 {
   int bit0, bit1;
 
-  /* ??? Fixme: should work on CONST_DOUBLE as well.  */
+  /* ??? Fixme: should work on CONST_WIDE_INT as well.  */
   if (GET_CODE (op1) != CONST_INT || GET_CODE (op2) != CONST_INT)
     return VOIDmode;
 
@@ -3355,6 +3355,7 @@ s390_rtx_costs (rtx x, machine_mode mode, int outer_code,
     case LABEL_REF:
     case SYMBOL_REF:
     case CONST_DOUBLE:
+    case CONST_WIDE_INT:
     case MEM:
       *total = 0;
       return true;
@@ -3662,7 +3663,7 @@ tls_symbolic_reference_mentioned_p (rtx op)
 
 /* Return true if OP is a legitimate general operand when
    generating PIC code.  It is given that flag_pic is on
-   and that OP satisfies CONSTANT_P or is a CONST_DOUBLE.  */
+   and that OP satisfies CONSTANT_P.  */
 
 int
 legitimate_pic_operand_p (rtx op)
@@ -3677,7 +3678,7 @@ legitimate_pic_operand_p (rtx op)
 }
 
 /* Returns true if the constant value OP is a legitimate general operand.
-   It is given that OP satisfies CONSTANT_P or is a CONST_DOUBLE.  */
+   It is given that OP satisfies CONSTANT_P.  */
 
 static bool
 s390_legitimate_constant_p (machine_mode mode, rtx op)
@@ -3731,6 +3732,7 @@ s390_cannot_force_const_mem (machine_mode mode, rtx x)
     {
     case CONST_INT:
     case CONST_DOUBLE:
+    case CONST_WIDE_INT:
     case CONST_VECTOR:
       /* Accept all non-symbolic constants.  */
       return false;
@@ -3831,8 +3833,9 @@ legitimate_reload_constant_p (rtx op)
     return true;
 
   /* Accept double-word operands that can be split.  */
-  if (GET_CODE (op) == CONST_INT
-      && trunc_int_for_mode (INTVAL (op), word_mode) != INTVAL (op))
+  if (GET_CODE (op) == CONST_WIDE_INT
+      || (GET_CODE (op) == CONST_INT
+	  && trunc_int_for_mode (INTVAL (op), word_mode) != INTVAL (op)))
     {
       machine_mode dword_mode = word_mode == SImode ? DImode : TImode;
       rtx hi = operand_subword (op, 0, 0, dword_mode);
@@ -3896,6 +3899,7 @@ s390_preferred_reload_class (rtx op, reg_class_t rclass)
       case CONST_VECTOR:
       case CONST_DOUBLE:
       case CONST_INT:
+      case CONST_WIDE_INT:
 	if (reg_class_subset_p (GENERAL_REGS, rclass)
 	    && legitimate_reload_constant_p (op))
 	  return GENERAL_REGS;
@@ -4047,6 +4051,7 @@ s390_reload_symref_address (rtx reg, rtx mem, rtx scratch, bool tomem)
   /* Reload might have pulled a constant out of the literal pool.
      Force it back in.  */
   if (CONST_INT_P (mem) || GET_CODE (mem) == CONST_DOUBLE
+      || GET_CODE (mem) == CONST_WIDE_INT
       || GET_CODE (mem) == CONST_VECTOR
       || GET_CODE (mem) == CONST)
     mem = force_const_mem (GET_MODE (reg), mem);
@@ -7233,15 +7238,16 @@ print_operand (FILE *file, rtx x, int code)
       fprintf (file, HOST_WIDE_INT_PRINT_DEC, ival);
       break;
 
-    case CONST_DOUBLE:
-      gcc_assert (GET_MODE (x) == VOIDmode);
+    case CONST_WIDE_INT:
       if (code == 'b')
-        fprintf (file, HOST_WIDE_INT_PRINT_DEC, CONST_DOUBLE_LOW (x) & 0xff);
+        fprintf (file, HOST_WIDE_INT_PRINT_DEC,
+		 CONST_WIDE_INT_ELT (x, 0) & 0xff);
       else if (code == 'x')
-        fprintf (file, HOST_WIDE_INT_PRINT_DEC, CONST_DOUBLE_LOW (x) & 0xffff);
+        fprintf (file, HOST_WIDE_INT_PRINT_DEC,
+		 CONST_WIDE_INT_ELT (x, 0) & 0xffff);
       else if (code == 'h')
         fprintf (file, HOST_WIDE_INT_PRINT_DEC,
-		 ((CONST_DOUBLE_LOW (x) & 0xffff) ^ 0x8000) - 0x8000);
+		 ((CONST_WIDE_INT_ELT (x, 0) & 0xffff) ^ 0x8000) - 0x8000);
       else
 	{
 	  if (code == 0)
diff --git a/gcc/config/s390/s390.h b/gcc/config/s390/s390.h
index b96549e..52ed7a4 100644
--- a/gcc/config/s390/s390.h
+++ b/gcc/config/s390/s390.h
@@ -155,6 +155,8 @@ enum processor_flags
 #define SWITCHABLE_TARGET 1
 #endif
 
+#define TARGET_SUPPORTS_WIDE_INT 1
+
 /* Use the ABI introduced with IBM z13:
    - pass vector arguments <= 16 bytes in VRs
    - align *all* vector types to 8 bytes  */
-- 
2.3.0

Re: [PATCH] S/390: Wide int support.

[Ulrich Weigand]

Dominik Vogt wrote:

> +; Note: Although CONST_INT and CONST_DOUBLE are not handled in this predicate,
> +; at least one of them needs to appear or otherwise safe_predicate_mode will
> +; assume that a DImode LABEL_REF is not accepted either (see genrecog.c).

The problem is not DImode LABEL_REFs, but rather VOIDmode LABEL_REFs when
matched against a match_operand:DI.

Otherwise, this patch is OK.

Thanks,
Ulrich

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

Re: [PATCH] S/390: Wide int support.

[Andreas Krebbel]

On 12/11/2015 03:20 PM, Dominik Vogt wrote:
> The attached patch introduces wide int support to S/390 in order
> to resolve a test case failure in gcc.dg/pr68129_1.c that is
> caused by an assertion in
> simplify-rtx.c:simplify_const_binary_operation().

Applied with the change suggested by Uli.  Thanks!

-Andreas-

Re: [PATCH] S/390: Wide int support.

[Richard Sandiford]

"Ulrich Weigand" <uweigand@de.ibm.com> writes:
> Dominik Vogt wrote:
>
>> +; Note: Although CONST_INT and CONST_DOUBLE are not handled in this
>> predicate,
>> +; at least one of them needs to appear or otherwise safe_predicate_mode will
>> +; assume that a DImode LABEL_REF is not accepted either (see genrecog.c).
>
> The problem is not DImode LABEL_REFs, but rather VOIDmode LABEL_REFs when
> matched against a match_operand:DI.

It'd be good to fix this in a more direct way though, rather than
hack around it.  It's possible that the trick will stop working
if genrecog.c gets smarter.

When do label_refs have VOIDmode?  Is this an m31-ism?

Thanks,
Richard

Re: [PATCH] S/390: Wide int support.

[Ulrich Weigand]

Richard Sandiford wrote:
> "Ulrich Weigand" <uweigand@de.ibm.com> writes:
> > The problem is not DImode LABEL_REFs, but rather VOIDmode LABEL_REFs when
> > matched against a match_operand:DI.
> 
> It'd be good to fix this in a more direct way though, rather than
> hack around it.  It's possible that the trick will stop working
> if genrecog.c gets smarter.
> 
> When do label_refs have VOIDmode?  Is this an m31-ism?

No, this seems to be a cross-platform issue.  For one, RTX in .md files
pretty much consistently uses (label_ref ...) without a mode.  This means
that any LABEL_REFs generated from .md file expanders or splitters will
use VOIDmode.

For LABEL_REFs generated via explicit gen_rtx_LABEL_REF, usage seems to
be mixed between using VOIDmode and Pmode in target C++ files.  Common
code does seem to be using always Pmode, as far as I can see.

Are LABEL_REFs in fact supposed to always have Pmode?

Bye,
Ulrich

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

Re: [PATCH] S/390: Wide int support.

[Richard Sandiford]

"Ulrich Weigand" <uweigand@de.ibm.com> writes:
> Richard Sandiford wrote:
>> "Ulrich Weigand" <uweigand@de.ibm.com> writes:
>> > The problem is not DImode LABEL_REFs, but rather VOIDmode LABEL_REFs when
>> > matched against a match_operand:DI.
>> 
>> It'd be good to fix this in a more direct way though, rather than
>> hack around it.  It's possible that the trick will stop working
>> if genrecog.c gets smarter.
>> 
>> When do label_refs have VOIDmode?  Is this an m31-ism?
>
> No, this seems to be a cross-platform issue.  For one, RTX in .md files
> pretty much consistently uses (label_ref ...) without a mode.  This means
> that any LABEL_REFs generated from .md file expanders or splitters will
> use VOIDmode.
>
> For LABEL_REFs generated via explicit gen_rtx_LABEL_REF, usage seems to
> be mixed between using VOIDmode and Pmode in target C++ files.  Common
> code does seem to be using always Pmode, as far as I can see.
>
> Are LABEL_REFs in fact supposed to always have Pmode?

Yeah, I think so, or at least always be Pmode or ptr_mode.  It sounds from
what you say that reality doesn't match though.

We should probably fix the remaining gen_rtx_LABEL_REF calls and get the
expanders to automatically add modes where needed, but that clearly
isn't stage 3 material.

Thanks,
Richard