diff mbox

wide-int, rs6000

Message ID 52206F9F-3BFD-44E5-91AA-C4250D9C681C@comcast.net
State New
Headers show

Commit Message

Mike Stump Nov. 26, 2013, 10:46 p.m. UTC
On Nov 25, 2013, at 12:03 PM, David Edelsohn <dje.gcc@gmail.com> wrote:
> Thanks for doing this conversion work.  A few questions and comments:
> 
> 1) Because rs6000 is one of the few ports that was completely
> converted to wide-int instead of simply accommodating wide-int, what
> is the compile-time performance impact of this conversion?

Others are interested in time, so I will see about characterizing it for folks.

> 2) non_logical_cint_operand changed const_double to const_wide_int, it
> did not add the additional CODE. Mike explained why in a private
> conversation, but the ChangeLog should be corrected.

Fixed.

> 3) altivec_resolve_overloaded_builtin, both hunks should be converted
> the same way, using tree_fits_uhwi_p
> 
> -   && TREE_CODE (arg2) == INTEGER_CST
> -   && TREE_INT_CST_HIGH (arg2) == 0
> -   && (TREE_INT_CST_LOW (arg2) == 0 || TREE_INT_CST_LOW (arg2) == 1))
> +   && tree_fits_uhwi_p (arg2)
> +   && wi::ltu_p (arg2, 2))

Richard suggested for speed that we go the other direction.  I agreed, what is what I'll include here for your consideration.

> 4) easy_altivec_constant, the comment about 32 bit should be removed
> because wide-int should remove the dependency on 32 bit vs 64 bit host
> wide int.

Fixed.

> 5) rs6000_aggregate_candidate, is this change correct for Ada and
> non-constant type size?

No, it was an oversight, fixed.

> The rest looks good. I'd like to see the revised patch before approving.

Ok?
* config/rs6000/predicates.md
	(any_operand): Add const_wide_int.
	(zero_constant): Likewise.
	(input_operand): Likewise.
	(splat_input_operand): Likewise.
	(non_logical_cint_operand): Change const_double to const_wide_int.
	* config/rs6000/rs6000.c
	(num_insns_constant): Handle CONST_WIDE_INT.
	(easy_altivec_constant): Remove comment.
	(paired_expand_vector_init): Use CONSTANT_P.
	(rs6000_legitimize_address): Handle CONST_WIDE_INT.
	(rs6000_emit_move): Update checks.
	(rs6000_aggregate_candidate): Use wide-int interfaces.
	(rs6000_expand_ternop_builtin): Likewise.
	(rs6000_output_move_128bit): Handle CONST_WIDE_INT.
	(rs6000_assemble_integer): Likewise.
	(rs6000_hash_constant): Likewise.
	(output_toc): Likewise.
	(rs6000_rtx_costs): Likewise.
	(rs6000_emit_swrsqrt); Update call to real_from_integer.
	* config/rs6000/rs6000-c.c: Include wide-int.h.
	(altivec_resolve_overloaded_builtin): Use wide-int interfaces.
	* config/rs6000/rs6000.h
	(TARGET_SUPPORTS_WIDE_INT): New.
	* config/rs6000/rs6000.md
	Use const_scalar_int_operand.  Handle CONST_WIDE_INT.

Comments

David Edelsohn Nov. 27, 2013, 12:17 a.m. UTC | #1
On Tue, Nov 26, 2013 at 5:46 PM, Mike Stump <mikestump@comcast.net> wrote:

> Ok?

The revised version of the patch looks okay from a technical
perspective, but I still am concerned about the compile-time impact. I
thought that I saw a comment on IRC that wide-int seems to have a 3-5%
compile time impact, which is a concern. I am concerned about the
rs6000 port being further slowed with respect to other targets.

Thanks, David
Kenneth Zadeck Nov. 27, 2013, 1:07 a.m. UTC | #2
We will of course measure it but the only thing that is different because of the conversion is that timode integers are packaged differently



> On Nov 26, 2013, at 6:17 PM, David Edelsohn <dje.gcc@gmail.com> wrote:
> 
>> On Tue, Nov 26, 2013 at 5:46 PM, Mike Stump <mikestump@comcast.net> wrote:
>> 
>> Ok?
> 
> The revised version of the patch looks okay from a technical
> perspective, but I still am concerned about the compile-time impact. I
> thought that I saw a comment on IRC that wide-int seems to have a 3-5%
> compile time impact, which is a concern. I am concerned about the
> rs6000 port being further slowed with respect to other targets.
> 
> Thanks, David
Richard Biener Nov. 27, 2013, 11:31 a.m. UTC | #3
On Wed, Nov 27, 2013 at 2:07 AM, Kenneth Zadeck
<zadeck@naturalbridge.com> wrote:
> We will of course measure it but the only thing that is different because of the conversion is that timode integers are packaged differently

Yeah, the slowness is due to generic code modification, not a port
using CONST_WIDE_INT or not.

Richard.

>
>
>> On Nov 26, 2013, at 6:17 PM, David Edelsohn <dje.gcc@gmail.com> wrote:
>>
>>> On Tue, Nov 26, 2013 at 5:46 PM, Mike Stump <mikestump@comcast.net> wrote:
>>>
>>> Ok?
>>
>> The revised version of the patch looks okay from a technical
>> perspective, but I still am concerned about the compile-time impact. I
>> thought that I saw a comment on IRC that wide-int seems to have a 3-5%
>> compile time impact, which is a concern. I am concerned about the
>> rs6000 port being further slowed with respect to other targets.
>>
>> Thanks, David
diff mbox

Patch

diff --git a/gcc/config/rs6000/predicates.md b/gcc/config/rs6000/predicates.md
index c3118be..585df9a 100644
--- a/gcc/config/rs6000/predicates.md
+++ b/gcc/config/rs6000/predicates.md
@@ -19,7 +19,7 @@ 
 
 ;; Return 1 for anything except PARALLEL.
 (define_predicate "any_operand"
-  (match_code "const_int,const_double,const,symbol_ref,label_ref,subreg,reg,mem"))
+  (match_code "const_int,const_double,const_wide_int,const,symbol_ref,label_ref,subreg,reg,mem"))
 
 ;; Return 1 for any PARALLEL.
 (define_predicate "any_parallel_operand"
@@ -596,7 +596,7 @@ 
 
 ;; Return 1 if operand is constant zero (scalars and vectors).
 (define_predicate "zero_constant"
-  (and (match_code "const_int,const_double,const_vector")
+  (and (match_code "const_int,const_double,const_wide_int,const_vector")
        (match_test "op == CONST0_RTX (mode)")))
 
 ;; Return 1 if operand is 0.0.
@@ -790,7 +790,7 @@ 
 ;; Return 1 if op is a constant that is not a logical operand, but could
 ;; be split into one.
 (define_predicate "non_logical_cint_operand"
-  (and (match_code "const_int,const_double")
+  (and (match_code "const_int,const_wide_int")
        (and (not (match_operand 0 "logical_operand"))
 	    (match_operand 0 "reg_or_logical_cint_operand"))))
 
@@ -1059,7 +1059,7 @@ 
 ;; Return 1 if this operand is a valid input for a move insn.
 (define_predicate "input_operand"
   (match_code "symbol_ref,const,reg,subreg,mem,
-	       const_double,const_vector,const_int")
+	       const_double,const_wide_int,const_vector,const_int")
 {
   /* Memory is always valid.  */
   if (memory_operand (op, mode))
@@ -1072,8 +1072,7 @@ 
 
   /* Allow any integer constant.  */
   if (GET_MODE_CLASS (mode) == MODE_INT
-      && (GET_CODE (op) == CONST_INT
-	  || GET_CODE (op) == CONST_DOUBLE))
+      && CONST_SCALAR_INT_P (op))
     return 1;
 
   /* Allow easy vector constants.  */
@@ -1112,7 +1111,7 @@ 
 ;; Return 1 if this operand is a valid input for a vsx_splat insn.
 (define_predicate "splat_input_operand"
   (match_code "symbol_ref,const,reg,subreg,mem,
-	       const_double,const_vector,const_int")
+	       const_double,const_wide_int,const_vector,const_int")
 {
   if (MEM_P (op))
     {
diff --git a/gcc/config/rs6000/rs6000-c.c b/gcc/config/rs6000/rs6000-c.c
index 667e5a6..494b7bc 100644
--- a/gcc/config/rs6000/rs6000-c.c
+++ b/gcc/config/rs6000/rs6000-c.c
@@ -28,6 +28,7 @@ 
 #include "tree.h"
 #include "stor-layout.h"
 #include "stringpool.h"
+#include "wide-int.h"
 #include "c-family/c-common.h"
 #include "c-family/c-pragma.h"
 #include "diagnostic-core.h"
@@ -4208,8 +4209,7 @@  altivec_resolve_overloaded_builtin (location_t loc, tree fndecl,
       mode = TYPE_MODE (arg1_type);
       if ((mode == V2DFmode || mode == V2DImode) && VECTOR_MEM_VSX_P (mode)
 	  && TREE_CODE (arg2) == INTEGER_CST
-	  && TREE_INT_CST_HIGH (arg2) == 0
-	  && (TREE_INT_CST_LOW (arg2) == 0 || TREE_INT_CST_LOW (arg2) == 1))
+	  && wi::ltu_p (arg2, 2))
 	{
 	  tree call = NULL_TREE;
 
@@ -4294,8 +4294,7 @@  altivec_resolve_overloaded_builtin (location_t loc, tree fndecl,
       mode = TYPE_MODE (arg1_type);
       if ((mode == V2DFmode || mode == V2DImode) && VECTOR_UNIT_VSX_P (mode)
 	  && TREE_CODE (arg2) == INTEGER_CST
-	  && TREE_INT_CST_HIGH (arg2) == 0
-	  && (TREE_INT_CST_LOW (arg2) == 0 || TREE_INT_CST_LOW (arg2) == 1))
+	  && wi::ltu_p (arg2, 2))
 	{
 	  tree call = NULL_TREE;
 
diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
index add91c9..ffccecb 100644
--- a/gcc/config/rs6000/rs6000.c
+++ b/gcc/config/rs6000/rs6000.c
@@ -4871,6 +4871,15 @@  num_insns_constant (rtx op, enum machine_mode mode)
       else
 	return num_insns_constant_wide (INTVAL (op));
 
+    case CONST_WIDE_INT:
+      {
+	int i;
+	int ins = CONST_WIDE_INT_NUNITS (op) - 1;
+	for (i = 0; i < CONST_WIDE_INT_NUNITS (op); i++)
+	  ins += num_insns_constant_wide (CONST_WIDE_INT_ELT (op, i));
+	return ins;
+      }
+
       case CONST_DOUBLE:
 	if (mode == SFmode || mode == SDmode)
 	  {
@@ -5045,8 +5054,6 @@  easy_altivec_constant (rtx op, enum machine_mode mode)
 
   if (mode == V2DImode)
     {
-      /* In case the compiler is built 32-bit, CONST_DOUBLE constants are not
-	 easy.  */
       if (GET_CODE (CONST_VECTOR_ELT (op, 0)) != CONST_INT
 	  || GET_CODE (CONST_VECTOR_ELT (op, 1)) != CONST_INT)
 	return false;
@@ -5207,9 +5214,7 @@  paired_expand_vector_init (rtx target, rtx vals)
   for (i = 0; i < n_elts; ++i)
     {
       x = XVECEXP (vals, 0, i);
-      if (!(CONST_INT_P (x)
-	    || GET_CODE (x) == CONST_DOUBLE
-	    || GET_CODE (x) == CONST_FIXED))
+      if (!CONSTANT_P (x))
 	++n_var;
     }
   if (n_var == 0)
@@ -5361,9 +5366,7 @@  rs6000_expand_vector_init (rtx target, rtx vals)
   for (i = 0; i < n_elts; ++i)
     {
       x = XVECEXP (vals, 0, i);
-      if (!(CONST_INT_P (x)
-	    || GET_CODE (x) == CONST_DOUBLE
-	    || GET_CODE (x) == CONST_FIXED))
+      if (!CONSTANT_P (x))
 	++n_var, one_var = i;
       else if (x != CONST0_RTX (inner_mode))
 	all_const_zero = false;
@@ -6588,6 +6591,7 @@  rs6000_legitimize_address (rtx x, rtx oldx ATTRIBUTE_UNUSED,
 	   && TARGET_NO_TOC
 	   && ! flag_pic
 	   && GET_CODE (x) != CONST_INT
+	   && GET_CODE (x) != CONST_WIDE_INT
 	   && GET_CODE (x) != CONST_DOUBLE
 	   && CONSTANT_P (x)
 	   && GET_MODE_NUNITS (mode) == 1
@@ -8028,21 +8032,12 @@  rs6000_emit_move (rtx dest, rtx source, enum machine_mode mode)
     }
 
   /* Sanity checks.  Check that we get CONST_DOUBLE only when we should.  */
-  if (GET_CODE (operands[1]) == CONST_DOUBLE
-      && ! FLOAT_MODE_P (mode)
+  if (CONST_WIDE_INT_P (operands[1])
       && GET_MODE_BITSIZE (mode) <= HOST_BITS_PER_WIDE_INT)
     {
-      /* FIXME.  This should never happen.  */
-      /* Since it seems that it does, do the safe thing and convert
-	 to a CONST_INT.  */
-      operands[1] = gen_int_mode (CONST_DOUBLE_LOW (operands[1]), mode);
+      /* This should be fixed with the introduction of CONST_WIDE_INT.  */
+      gcc_unreachable ();
     }
-  gcc_assert (GET_CODE (operands[1]) != CONST_DOUBLE
-	      || FLOAT_MODE_P (mode)
-	      || ((CONST_DOUBLE_HIGH (operands[1]) != 0
-		   || CONST_DOUBLE_LOW (operands[1]) < 0)
-		  && (CONST_DOUBLE_HIGH (operands[1]) != -1
-		      || CONST_DOUBLE_LOW (operands[1]) >= 0)));
 
   /* Check if GCC is setting up a block move that will end up using FP
      registers as temporaries.  We must make sure this is acceptable.  */
@@ -8557,8 +8552,10 @@  rs6000_aggregate_candidate (const_tree type, enum machine_mode *modep)
 	int count;
 	tree index = TYPE_DOMAIN (type);
 
-	/* Can't handle incomplete types.  */
-	if (!COMPLETE_TYPE_P (type))
+	/* Can't handle incomplete types nor sizes that are not
+	   fixed.  */
+	if (!COMPLETE_TYPE_P (type)
+	    || TREE_CODE (TYPE_SIZE (type)) != INTEGER_CST)
 	  return -1;
 
 	count = rs6000_aggregate_candidate (TREE_TYPE (type), modep);
@@ -8575,9 +8572,7 @@  rs6000_aggregate_candidate (const_tree type, enum machine_mode *modep)
 		      - tree_to_uhwi (TYPE_MIN_VALUE (index)));
 
 	/* There must be no padding.  */
-	if (!tree_fits_uhwi_p (TYPE_SIZE (type))
-	    || ((HOST_WIDE_INT) tree_to_uhwi (TYPE_SIZE (type))
-		!= count * GET_MODE_BITSIZE (*modep)))
+	if (wi::ne_p (TYPE_SIZE (type), count * GET_MODE_BITSIZE (*modep)))
 	  return -1;
 
 	return count;
@@ -8589,8 +8584,10 @@  rs6000_aggregate_candidate (const_tree type, enum machine_mode *modep)
 	int sub_count;
 	tree field;
 
-	/* Can't handle incomplete types.  */
-	if (!COMPLETE_TYPE_P (type))
+	/* Can't handle incomplete types nor sizes that are not
+	   fixed.  */
+	if (!COMPLETE_TYPE_P (type)
+	    || TREE_CODE (TYPE_SIZE (type)) != INTEGER_CST)
 	  return -1;
 
 	for (field = TYPE_FIELDS (type); field; field = TREE_CHAIN (field))
@@ -8605,9 +8602,7 @@  rs6000_aggregate_candidate (const_tree type, enum machine_mode *modep)
 	  }
 
 	/* There must be no padding.  */
-	if (!tree_fits_uhwi_p (TYPE_SIZE (type))
-	    || ((HOST_WIDE_INT) tree_to_uhwi (TYPE_SIZE (type))
-		!= count * GET_MODE_BITSIZE (*modep)))
+	if (wi::ne_p (TYPE_SIZE (type), count * GET_MODE_BITSIZE (*modep)))
 	  return -1;
 
 	return count;
@@ -8621,9 +8616,10 @@  rs6000_aggregate_candidate (const_tree type, enum machine_mode *modep)
 	int sub_count;
 	tree field;
 
-	/* Can't handle incomplete types.  */
-	if (!COMPLETE_TYPE_P (type))
-	  return -1;
+	/* Can't handle incomplete types nor sizes that are not
+	   fixed.  */
+	if (!COMPLETE_TYPE_P (type)
+	    || TREE_CODE (TYPE_SIZE (type)) != INTEGER_CST)
 
 	for (field = TYPE_FIELDS (type); field; field = TREE_CHAIN (field))
 	  {
@@ -8637,9 +8633,7 @@  rs6000_aggregate_candidate (const_tree type, enum machine_mode *modep)
 	  }
 
 	/* There must be no padding.  */
-	if (!tree_fits_uhwi_p (TYPE_SIZE (type))
-	    || ((HOST_WIDE_INT) tree_to_uhwi (TYPE_SIZE (type))
-		!= count * GET_MODE_BITSIZE (*modep)))
+	if (wi::ne_p (TYPE_SIZE (type), count * GET_MODE_BITSIZE (*modep)))
 	  return -1;
 
 	return count;
@@ -12152,16 +12146,14 @@  rs6000_expand_ternop_builtin (enum insn_code icode, tree exp, rtx target)
       /* Check whether the 2nd and 3rd arguments are integer constants and in
 	 range and prepare arguments.  */
       STRIP_NOPS (arg1);
-      if (TREE_CODE (arg1) != INTEGER_CST
-	  || !IN_RANGE (TREE_INT_CST_LOW (arg1), 0, 1))
+      if (TREE_CODE (arg1) != INTEGER_CST || wi::geu_p (arg1, 2))
 	{
 	  error ("argument 2 must be 0 or 1");
 	  return const0_rtx;
 	}
 
       STRIP_NOPS (arg2);
-      if (TREE_CODE (arg2) != INTEGER_CST
-	  || !IN_RANGE (TREE_INT_CST_LOW (arg2), 0, 15))
+      if (TREE_CODE (arg2) != INTEGER_CST || wi::geu_p (arg1, 16))
 	{
 	  error ("argument 3 must be in the range 0..15");
 	  return const0_rtx;
@@ -16883,6 +16875,7 @@  rs6000_output_move_128bit (rtx operands[])
   /* Constants.  */
   else if (dest_regno >= 0
 	   && (GET_CODE (src) == CONST_INT
+	       || GET_CODE (src) == CONST_WIDE_INT
 	       || GET_CODE (src) == CONST_DOUBLE
 	       || GET_CODE (src) == CONST_VECTOR))
     {
@@ -17897,8 +17890,7 @@  rs6000_assemble_integer (rtx x, unsigned int size, int aligned_p)
       if (TARGET_RELOCATABLE
 	  && in_section != toc_section
 	  && !recurse
-	  && GET_CODE (x) != CONST_INT
-	  && GET_CODE (x) != CONST_DOUBLE
+	  && !CONST_SCALAR_INT_P (x)
 	  && CONSTANT_P (x))
 	{
 	  char buf[256];
@@ -24645,6 +24637,15 @@  rs6000_hash_constant (rtx k)
     case LABEL_REF:
       return result * 1231 + (unsigned) INSN_UID (XEXP (k, 0));
 
+    case CONST_WIDE_INT:
+      {
+	int i;
+	flen = CONST_WIDE_INT_NUNITS (k);
+	for (i = 0; i < flen; i++)
+	  result = result * 613 + CONST_WIDE_INT_ELT (k, i);
+	return result;
+      }
+
     case CONST_DOUBLE:
       if (mode != VOIDmode)
 	return real_hash (CONST_DOUBLE_REAL_VALUE (k)) * result;
@@ -24849,7 +24850,7 @@  output_toc (FILE *file, rtx x, int labelno, enum machine_mode mode)
 
   /* If we're going to put a double constant in the TOC, make sure it's
      aligned properly when strict alignment is on.  */
-  if (GET_CODE (x) == CONST_DOUBLE
+  if ((CONST_DOUBLE_P (x) || CONST_WIDE_INT_P (x))
       && STRICT_ALIGNMENT
       && GET_MODE_BITSIZE (mode) >= 64
       && ! (TARGET_NO_FP_IN_TOC && ! TARGET_MINIMAL_TOC)) {
@@ -28837,6 +28838,7 @@  rs6000_rtx_costs (rtx x, int code, int outer_code, int opno ATTRIBUTE_UNUSED,
       /* FALLTHRU */
 
     case CONST_DOUBLE:
+    case CONST_WIDE_INT:
     case CONST:
     case HIGH:
     case SYMBOL_REF:
@@ -29476,7 +29478,7 @@  rs6000_emit_swrsqrt (rtx dst, rtx src)
   gcc_assert (code != CODE_FOR_nothing);
 
   /* Load up the constant 1.5 either as a scalar, or as a vector.  */
-  real_from_integer (&dconst3_2, VOIDmode, 3, 0, 0);
+  real_from_integer (&dconst3_2, VOIDmode, 3, SIGNED);
   SET_REAL_EXP (&dconst3_2, REAL_EXP (&dconst3_2) - 1);
 
   halfthree = rs6000_load_constant_and_splat (mode, dconst3_2);
diff --git a/gcc/config/rs6000/rs6000.h b/gcc/config/rs6000/rs6000.h
index eb59235..861fa02 100644
--- a/gcc/config/rs6000/rs6000.h
+++ b/gcc/config/rs6000/rs6000.h
@@ -2650,3 +2650,4 @@  enum rs6000_builtin_type_index
 extern GTY(()) tree rs6000_builtin_types[RS6000_BTI_MAX];
 extern GTY(()) tree rs6000_builtin_decls[RS6000_BUILTIN_COUNT];
 
+#define TARGET_SUPPORTS_WIDE_INT 1
diff --git a/gcc/config/rs6000/rs6000.md b/gcc/config/rs6000/rs6000.md
index 4de7c3a..089a229 100644
--- a/gcc/config/rs6000/rs6000.md
+++ b/gcc/config/rs6000/rs6000.md
@@ -10231,7 +10231,7 @@ 
 
 (define_split
   [(set (match_operand:DI 0 "gpc_reg_operand" "")
-	(match_operand:DI 1 "const_double_operand" ""))]
+	(match_operand:DI 1 "const_scalar_int_operand" ""))]
   "TARGET_POWERPC64 && num_insns_constant (operands[1], DImode) > 1"
   [(set (match_dup 0) (match_dup 2))
    (set (match_dup 0) (plus:DI (match_dup 0) (match_dup 3)))]
@@ -10297,7 +10297,7 @@ 
 
 (define_split
   [(set (match_operand:TI2 0 "int_reg_operand" "")
-	(match_operand:TI2 1 "const_double_operand" ""))]
+	(match_operand:TI2 1 "const_scalar_int_operand" ""))]
   "TARGET_POWERPC64
    && (VECTOR_MEM_NONE_P (<MODE>mode)
        || (reload_completed && INT_REGNO_P (REGNO (operands[0]))))"
@@ -10309,12 +10309,12 @@ 
 				       <MODE>mode);
   operands[3] = operand_subword_force (operands[0], WORDS_BIG_ENDIAN != 0,
 				       <MODE>mode);
-  if (GET_CODE (operands[1]) == CONST_DOUBLE)
+  if (CONST_WIDE_INT_P (operands[1]))
     {
-      operands[4] = GEN_INT (CONST_DOUBLE_HIGH (operands[1]));
-      operands[5] = GEN_INT (CONST_DOUBLE_LOW (operands[1]));
+      operands[4] = GEN_INT (CONST_WIDE_INT_ELT (operands[1], 1));
+      operands[5] = GEN_INT (CONST_WIDE_INT_ELT (operands[1], 0));
     }
-  else if (GET_CODE (operands[1]) == CONST_INT)
+  else if (CONST_INT_P (operands[1]))
     {
       operands[4] = GEN_INT (- (INTVAL (operands[1]) < 0));
       operands[5] = operands[1];