[PATCH] Fix up go regressions caused by my recent switchconv changes (PR go/91617)

[PATCH] Fix up go regressions caused by my recent switchconv changes (PR go/91617)

[Jakub Jelinek]

Hi!

Apparently my recent tree-{cfg,switch-conversion}.c changes broke a bunch of
go tests.
The problem is that range_check_type actually doesn't guarantee an unsigned
type; it forces integer type for enum/bool (that was what was really needed
to fix the PR), and for integer types that don't wrap forces unsigned type
(and then verifies the wrap-around).  Seems go uses -fwrapv by default
and we got thus signed types from it.  That is fine if we emit the
x >= low && x < high range tests as x - low >= 0 && x - low < high - low,
but we actually don't emit the >= 0 check and so we do need an unsigned
type.  The other uses of range_check_type also eventually also call
unsigned_type_for, e.g.
  etype = range_check_type (etype);
  if (etype == NULL_TREE)
    return NULL_TREE;
     
  if (POINTER_TYPE_P (etype))
    etype = unsigned_type_for (etype);
and in the recursion then because low is 0:
      if (! TYPE_UNSIGNED (etype))          
        {
          etype = unsigned_type_for (etype);
          high = fold_convert_loc (loc, etype, high);
          exp = fold_convert_loc (loc, etype, exp);
        }
Similarly match.pd:
        tree etype = range_check_type (TREE_TYPE (@0));
        if (etype)
          {
            if (! TYPE_UNSIGNED (etype))
              etype = unsigned_type_for (etype);
...
So, the following patch calls unsigned_type_for on the range_check_type
result.

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

2019-08-31  Jakub Jelinek  <jakub@redhat.com>

	PR go/91617
	* tree-cfg.c (generate_range_test): Call unsigned_type_for on
	the range_check_type result.
	* tree-switch-conversion.c (switch_conversion::build_one_array,
	bit_test_cluster::emit): Likewise.

--- gcc/tree-cfg.c.jj	2019-08-31 12:09:09.135153318 +0200
+++ gcc/tree-cfg.c	2019-08-31 12:48:06.259939680 +0200
@@ -9222,6 +9222,7 @@ generate_range_test (basic_block bb, tre
 {
   tree type = TREE_TYPE (index);
   tree utype = range_check_type (type);
+  utype = unsigned_type_for (utype);
 
   low = fold_convert (utype, low);
   high = fold_convert (utype, high);
--- gcc/tree-switch-conversion.c.jj	2019-08-31 12:09:09.129153406 +0200
+++ gcc/tree-switch-conversion.c	2019-08-31 12:48:28.340617487 +0200
@@ -616,6 +616,7 @@ switch_conversion::build_one_array (int
 
       /* We must use type of constructor values.  */
       gimple_seq seq = NULL;
+      type = unsigned_type_for (type);
       tree tmp = gimple_convert (&seq, type, m_index_expr);
       tree tmp2 = gimple_build (&seq, MULT_EXPR, type,
 				wide_int_to_tree (type, coeff_a), tmp);
@@ -1486,6 +1487,7 @@ bit_test_cluster::emit (tree index_expr,
   unsigned int count;
 
   tree unsigned_index_type = range_check_type (index_type);
+  unsigned_index_type = unsigned_type_for (unsigned_index_type);
 
   gimple_stmt_iterator gsi;
   gassign *shift_stmt;

	Jakub

Re: [PATCH] Fix up go regressions caused by my recent switchconv changes (PR go/91617)

[Ian Lance Taylor via gcc-patches]

On Sat, Aug 31, 2019 at 6:12 AM Jakub Jelinek <jakub@redhat.com> wrote:
>
> Hi!
>
> Apparently my recent tree-{cfg,switch-conversion}.c changes broke a bunch of
> go tests.
> The problem is that range_check_type actually doesn't guarantee an unsigned
> type; it forces integer type for enum/bool (that was what was really needed
> to fix the PR), and for integer types that don't wrap forces unsigned type
> (and then verifies the wrap-around).  Seems go uses -fwrapv by default
> and we got thus signed types from it.  That is fine if we emit the
> x >= low && x < high range tests as x - low >= 0 && x - low < high - low,
> but we actually don't emit the >= 0 check and so we do need an unsigned
> type.  The other uses of range_check_type also eventually also call
> unsigned_type_for, e.g.
>   etype = range_check_type (etype);
>   if (etype == NULL_TREE)
>     return NULL_TREE;
>
>   if (POINTER_TYPE_P (etype))
>     etype = unsigned_type_for (etype);
> and in the recursion then because low is 0:
>       if (! TYPE_UNSIGNED (etype))
>         {
>           etype = unsigned_type_for (etype);
>           high = fold_convert_loc (loc, etype, high);
>           exp = fold_convert_loc (loc, etype, exp);
>         }
> Similarly match.pd:
>         tree etype = range_check_type (TREE_TYPE (@0));
>         if (etype)
>           {
>             if (! TYPE_UNSIGNED (etype))
>               etype = unsigned_type_for (etype);
> ...
> So, the following patch calls unsigned_type_for on the range_check_type
> result.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
>
> 2019-08-31  Jakub Jelinek  <jakub@redhat.com>
>
>         PR go/91617
>         * tree-cfg.c (generate_range_test): Call unsigned_type_for on
>         the range_check_type result.
>         * tree-switch-conversion.c (switch_conversion::build_one_array,
>         bit_test_cluster::emit): Likewise.
>
> --- gcc/tree-cfg.c.jj   2019-08-31 12:09:09.135153318 +0200
> +++ gcc/tree-cfg.c      2019-08-31 12:48:06.259939680 +0200
> @@ -9222,6 +9222,7 @@ generate_range_test (basic_block bb, tre
>  {
>    tree type = TREE_TYPE (index);
>    tree utype = range_check_type (type);
> +  utype = unsigned_type_for (utype);
>
>    low = fold_convert (utype, low);
>    high = fold_convert (utype, high);
> --- gcc/tree-switch-conversion.c.jj     2019-08-31 12:09:09.129153406 +0200
> +++ gcc/tree-switch-conversion.c        2019-08-31 12:48:28.340617487 +0200
> @@ -616,6 +616,7 @@ switch_conversion::build_one_array (int
>
>        /* We must use type of constructor values.  */
>        gimple_seq seq = NULL;
> +      type = unsigned_type_for (type);
>        tree tmp = gimple_convert (&seq, type, m_index_expr);
>        tree tmp2 = gimple_build (&seq, MULT_EXPR, type,
>                                 wide_int_to_tree (type, coeff_a), tmp);
> @@ -1486,6 +1487,7 @@ bit_test_cluster::emit (tree index_expr,
>    unsigned int count;
>
>    tree unsigned_index_type = range_check_type (index_type);
> +  unsigned_index_type = unsigned_type_for (unsigned_index_type);
>
>    gimple_stmt_iterator gsi;
>    gassign *shift_stmt;



Thanks for looking into this.

Ian

Re: [PATCH] Fix up go regressions caused by my recent switchconv changes (PR go/91617)

[Richard Biener]

On August 31, 2019 3:12:07 PM GMT+02:00, Jakub Jelinek <jakub@redhat.com> wrote:
>Hi!
>
>Apparently my recent tree-{cfg,switch-conversion}.c changes broke a
>bunch of
>go tests.
>The problem is that range_check_type actually doesn't guarantee an
>unsigned
>type; it forces integer type for enum/bool (that was what was really
>needed
>to fix the PR), and for integer types that don't wrap forces unsigned
>type
>(and then verifies the wrap-around).  Seems go uses -fwrapv by default
>and we got thus signed types from it.  That is fine if we emit the
>x >= low && x < high range tests as x - low >= 0 && x - low < high -
>low,
>but we actually don't emit the >= 0 check and so we do need an unsigned
>type.  The other uses of range_check_type also eventually also call
>unsigned_type_for, e.g.
>  etype = range_check_type (etype);
>  if (etype == NULL_TREE)
>    return NULL_TREE;
>     
>  if (POINTER_TYPE_P (etype))
>    etype = unsigned_type_for (etype);
>and in the recursion then because low is 0:
>      if (! TYPE_UNSIGNED (etype))          
>        {
>          etype = unsigned_type_for (etype);
>          high = fold_convert_loc (loc, etype, high);
>          exp = fold_convert_loc (loc, etype, exp);
>        }
>Similarly match.pd:
>        tree etype = range_check_type (TREE_TYPE (@0));
>        if (etype)
>          {
>            if (! TYPE_UNSIGNED (etype))
>              etype = unsigned_type_for (etype);
>...
>So, the following patch calls unsigned_type_for on the range_check_type
>result.
>
>Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

Hmm, couldn't we make range_check_type_for take an argument whether signed or unsigned type is required? That is, what do we do if the caller wants a signed type? Leaving it unspecified what the function returns is odd. 

Richard. 

>2019-08-31  Jakub Jelinek  <jakub@redhat.com>
>
>	PR go/91617
>	* tree-cfg.c (generate_range_test): Call unsigned_type_for on
>	the range_check_type result.
>	* tree-switch-conversion.c (switch_conversion::build_one_array,
>	bit_test_cluster::emit): Likewise.
>
>--- gcc/tree-cfg.c.jj	2019-08-31 12:09:09.135153318 +0200
>+++ gcc/tree-cfg.c	2019-08-31 12:48:06.259939680 +0200
>@@ -9222,6 +9222,7 @@ generate_range_test (basic_block bb, tre
> {
>   tree type = TREE_TYPE (index);
>   tree utype = range_check_type (type);
>+  utype = unsigned_type_for (utype);
> 
>   low = fold_convert (utype, low);
>   high = fold_convert (utype, high);
>--- gcc/tree-switch-conversion.c.jj	2019-08-31 12:09:09.129153406 +0200
>+++ gcc/tree-switch-conversion.c	2019-08-31 12:48:28.340617487 +0200
>@@ -616,6 +616,7 @@ switch_conversion::build_one_array (int
> 
>       /* We must use type of constructor values.  */
>       gimple_seq seq = NULL;
>+      type = unsigned_type_for (type);
>       tree tmp = gimple_convert (&seq, type, m_index_expr);
>       tree tmp2 = gimple_build (&seq, MULT_EXPR, type,
> 				wide_int_to_tree (type, coeff_a), tmp);
>@@ -1486,6 +1487,7 @@ bit_test_cluster::emit (tree index_expr,
>   unsigned int count;
> 
>   tree unsigned_index_type = range_check_type (index_type);
>+  unsigned_index_type = unsigned_type_for (unsigned_index_type);
> 
>   gimple_stmt_iterator gsi;
>   gassign *shift_stmt;
>
>	Jakub

Re: [PATCH] Fix up go regressions caused by my recent switchconv changes (PR go/91617)

[Jakub Jelinek]

On Sat, Aug 31, 2019 at 07:23:35PM +0200, Richard Biener wrote:
> Hmm, couldn't we make range_check_type_for take an argument whether signed
> or unsigned type is required?  That is, what do we do if the caller wants
> a signed type?  Leaving it unspecified what the function returns is odd.

I think we never want specially signed type, either we don't care whether it
is signed or not, in that case we want just some type that wraps around, or we
do want an unsigned type.

So something like below, with using range_check_type (x, true) in some
places.

Note it will be more compile time expensive than just calling
unsigned_type_for after the range_check_type call, due to all the
wrap-around verification it does.

--- gcc/fold-const.h.jj	2019-08-08 08:34:28.010306157 +0200
+++ gcc/fold-const.h	2019-08-31 19:39:13.436366420 +0200
@@ -182,7 +182,7 @@ extern bool tree_expr_nonnegative_warnv_
 extern tree make_range (tree, int *, tree *, tree *, bool *);
 extern tree make_range_step (location_t, enum tree_code, tree, tree, tree,
 			     tree *, tree *, int *, bool *);
-extern tree range_check_type (tree);
+extern tree range_check_type (tree, bool = false);
 extern tree build_range_check (location_t, tree, tree, int, tree, tree);
 extern bool merge_ranges (int *, tree *, tree *, int, tree, tree, int,
 			  tree, tree);
--- gcc/fold-const.c.jj	2019-08-27 12:26:39.303884758 +0200
+++ gcc/fold-const.c	2019-08-31 19:39:06.235470312 +0200
@@ -4930,10 +4930,12 @@ maskable_range_p (const_tree low, const_
 }
 \f
 /* Helper routine for build_range_check and match.pd.  Return the type to
-   perform the check or NULL if it shouldn't be optimized.  */
+   perform the check or NULL if it shouldn't be optimized.
+   If UNSIGNEDP is true, the returned type must be unsigned, otherwise
+   it can be some INTEGER_TYPE that wraps around.  */
 
 tree
-range_check_type (tree etype)
+range_check_type (tree etype, bool unsignedp)
 {
   /* First make sure that arithmetics in this type is valid, then make sure
      that it wraps around.  */
@@ -4941,7 +4943,8 @@ range_check_type (tree etype)
     etype = lang_hooks.types.type_for_size (TYPE_PRECISION (etype),
 					    TYPE_UNSIGNED (etype));
 
-  if (TREE_CODE (etype) == INTEGER_TYPE && !TYPE_OVERFLOW_WRAPS (etype))
+  if (unsignedp
+      || (TREE_CODE (etype) == INTEGER_TYPE && !TYPE_OVERFLOW_WRAPS (etype)))
     {
       tree utype, minv, maxv;
 


	Jakub

Re: [PATCH] Fix up go regressions caused by my recent switchconv changes (PR go/91617)

[Richard Biener]

On August 31, 2019 7:41:17 PM GMT+02:00, Jakub Jelinek <jakub@redhat.com> wrote:
>On Sat, Aug 31, 2019 at 07:23:35PM +0200, Richard Biener wrote:
>> Hmm, couldn't we make range_check_type_for take an argument whether
>signed
>> or unsigned type is required?  That is, what do we do if the caller
>wants
>> a signed type?  Leaving it unspecified what the function returns is
>odd.
>
>I think we never want specially signed type, either we don't care
>whether it
>is signed or not, in that case we want just some type that wraps
>around, or we
>do want an unsigned type.

So why not always return an unsigned type then by telling type_for_size? 

>
>So something like below, with using range_check_type (x, true) in some
>places.
>
>Note it will be more compile time expensive than just calling
>unsigned_type_for after the range_check_type call, due to all the
>wrap-around verification it does.
>
>--- gcc/fold-const.h.jj	2019-08-08 08:34:28.010306157 +0200
>+++ gcc/fold-const.h	2019-08-31 19:39:13.436366420 +0200
>@@ -182,7 +182,7 @@ extern bool tree_expr_nonnegative_warnv_
> extern tree make_range (tree, int *, tree *, tree *, bool *);
>extern tree make_range_step (location_t, enum tree_code, tree, tree,
>tree,
> 			     tree *, tree *, int *, bool *);
>-extern tree range_check_type (tree);
>+extern tree range_check_type (tree, bool = false);
>extern tree build_range_check (location_t, tree, tree, int, tree,
>tree);
> extern bool merge_ranges (int *, tree *, tree *, int, tree, tree, int,
> 			  tree, tree);
>--- gcc/fold-const.c.jj	2019-08-27 12:26:39.303884758 +0200
>+++ gcc/fold-const.c	2019-08-31 19:39:06.235470312 +0200
>@@ -4930,10 +4930,12 @@ maskable_range_p (const_tree low, const_
> }
> \f>
>/* Helper routine for build_range_check and match.pd.  Return the type
>to
>-   perform the check or NULL if it shouldn't be optimized.  */
>+   perform the check or NULL if it shouldn't be optimized.
>+   If UNSIGNEDP is true, the returned type must be unsigned, otherwise
>+   it can be some INTEGER_TYPE that wraps around.  */
> 
> tree
>-range_check_type (tree etype)
>+range_check_type (tree etype, bool unsignedp)
> {
>/* First make sure that arithmetics in this type is valid, then make
>sure
>      that it wraps around.  */
>@@ -4941,7 +4943,8 @@ range_check_type (tree etype)
>     etype = lang_hooks.types.type_for_size (TYPE_PRECISION (etype),
> 					    TYPE_UNSIGNED (etype));
> 
>-  if (TREE_CODE (etype) == INTEGER_TYPE && !TYPE_OVERFLOW_WRAPS
>(etype))
>+  if (unsignedp
>+      || (TREE_CODE (etype) == INTEGER_TYPE && !TYPE_OVERFLOW_WRAPS
>(etype)))
>     {
>       tree utype, minv, maxv;
> 
>
>
>	Jakub

Re: [PATCH] Fix up go regressions caused by my recent switchconv changes (PR go/91617)

[Jakub Jelinek]

On Sat, Aug 31, 2019 at 08:25:49PM +0200, Richard Biener wrote:
> So why not always return an unsigned type then by telling type_for_size? 

So like this (if it passes bootstrap/regtest)?

2019-09-01  Jakub Jelinek  <jakub@redhat.com>

	PR go/91617
	* fold-const.c (range_check_type): For enumeral and boolean
	type, pass 1 to type_for_size langhook instead of
	TYPE_UNSIGNED (etype).  Return unsigned_type_for result whenever
	etype isn't TYPE_UNSIGNED INTEGER_TYPE.
	(build_range_check): Don't call unsigned_type_for for pointer types.
	* match.pd (X / C1 op C2): Don't call unsigned_type_for on
	range_check_type result.

--- gcc/fold-const.c.jj	2019-08-27 12:26:39.303884758 +0200
+++ gcc/fold-const.c	2019-09-01 18:22:41.866023675 +0200
@@ -4938,10 +4938,9 @@ range_check_type (tree etype)
   /* First make sure that arithmetics in this type is valid, then make sure
      that it wraps around.  */
   if (TREE_CODE (etype) == ENUMERAL_TYPE || TREE_CODE (etype) == BOOLEAN_TYPE)
-    etype = lang_hooks.types.type_for_size (TYPE_PRECISION (etype),
-					    TYPE_UNSIGNED (etype));
+    etype = lang_hooks.types.type_for_size (TYPE_PRECISION (etype), 1);
 
-  if (TREE_CODE (etype) == INTEGER_TYPE && !TYPE_OVERFLOW_WRAPS (etype))
+  if (TREE_CODE (etype) != INTEGER_TYPE || !TYPE_UNSIGNED (etype))
     {
       tree utype, minv, maxv;
 
@@ -5049,9 +5048,6 @@ build_range_check (location_t loc, tree
   if (etype == NULL_TREE)
     return NULL_TREE;
 
-  if (POINTER_TYPE_P (etype))
-    etype = unsigned_type_for (etype);
-
   high = fold_convert_loc (loc, etype, high);
   low = fold_convert_loc (loc, etype, low);
   exp = fold_convert_loc (loc, etype, exp);
--- gcc/match.pd.jj	2019-08-27 12:26:40.745863588 +0200
+++ gcc/match.pd	2019-09-01 18:23:02.098729356 +0200
@@ -1569,8 +1569,6 @@ (define_operator_list COND_TERNARY
 	tree etype = range_check_type (TREE_TYPE (@0));
 	if (etype)
 	  {
-	    if (! TYPE_UNSIGNED (etype))
-	      etype = unsigned_type_for (etype);
 	    hi = fold_convert (etype, hi);
 	    lo = fold_convert (etype, lo);
 	    hi = const_binop (MINUS_EXPR, etype, hi, lo);


	Jakub

Re: [PATCH] Fix up go regressions caused by my recent switchconv changes (PR go/91617)

[Richard Biener]

On September 1, 2019 6:34:25 PM GMT+02:00, Jakub Jelinek <jakub@redhat.com> wrote:
>On Sat, Aug 31, 2019 at 08:25:49PM +0200, Richard Biener wrote:
>> So why not always return an unsigned type then by telling
>type_for_size? 
>
>So like this (if it passes bootstrap/regtest)?

Yes. 

Thanks, 
Richard. 

>2019-09-01  Jakub Jelinek  <jakub@redhat.com>
>
>	PR go/91617
>	* fold-const.c (range_check_type): For enumeral and boolean
>	type, pass 1 to type_for_size langhook instead of
>	TYPE_UNSIGNED (etype).  Return unsigned_type_for result whenever
>	etype isn't TYPE_UNSIGNED INTEGER_TYPE.
>	(build_range_check): Don't call unsigned_type_for for pointer types.
>	* match.pd (X / C1 op C2): Don't call unsigned_type_for on
>	range_check_type result.
>
>--- gcc/fold-const.c.jj	2019-08-27 12:26:39.303884758 +0200
>+++ gcc/fold-const.c	2019-09-01 18:22:41.866023675 +0200
>@@ -4938,10 +4938,9 @@ range_check_type (tree etype)
>/* First make sure that arithmetics in this type is valid, then make
>sure
>      that it wraps around.  */
>if (TREE_CODE (etype) == ENUMERAL_TYPE || TREE_CODE (etype) ==
>BOOLEAN_TYPE)
>-    etype = lang_hooks.types.type_for_size (TYPE_PRECISION (etype),
>-					    TYPE_UNSIGNED (etype));
>+    etype = lang_hooks.types.type_for_size (TYPE_PRECISION (etype),
>1);
> 
>-  if (TREE_CODE (etype) == INTEGER_TYPE && !TYPE_OVERFLOW_WRAPS
>(etype))
>+  if (TREE_CODE (etype) != INTEGER_TYPE || !TYPE_UNSIGNED (etype))
>     {
>       tree utype, minv, maxv;
> 
>@@ -5049,9 +5048,6 @@ build_range_check (location_t loc, tree
>   if (etype == NULL_TREE)
>     return NULL_TREE;
> 
>-  if (POINTER_TYPE_P (etype))
>-    etype = unsigned_type_for (etype);
>-
>   high = fold_convert_loc (loc, etype, high);
>   low = fold_convert_loc (loc, etype, low);
>   exp = fold_convert_loc (loc, etype, exp);
>--- gcc/match.pd.jj	2019-08-27 12:26:40.745863588 +0200
>+++ gcc/match.pd	2019-09-01 18:23:02.098729356 +0200
>@@ -1569,8 +1569,6 @@ (define_operator_list COND_TERNARY
> 	tree etype = range_check_type (TREE_TYPE (@0));
> 	if (etype)
> 	  {
>-	    if (! TYPE_UNSIGNED (etype))
>-	      etype = unsigned_type_for (etype);
> 	    hi = fold_convert (etype, hi);
> 	    lo = fold_convert (etype, lo);
> 	    hi = const_binop (MINUS_EXPR, etype, hi, lo);
>
>
>	Jakub

Re: [PATCH] Fix up go regressions caused by my recent switchconv changes (PR go/91617)

[Jakub Jelinek]

On Sun, Sep 01, 2019 at 06:44:15PM +0200, Richard Biener wrote:
> On September 1, 2019 6:34:25 PM GMT+02:00, Jakub Jelinek <jakub@redhat.com> wrote:
> >On Sat, Aug 31, 2019 at 08:25:49PM +0200, Richard Biener wrote:
> >> So why not always return an unsigned type then by telling
> >type_for_size? 
> >
> >So like this (if it passes bootstrap/regtest)?
> 
> Yes. 

Unfortunately that didn't work, because TYPE_MAX_VALUE/TYPE_MIN_VALUE
are not present on POINTER_TYPEs.

Here is an updated version that passed bootstrap/regtest on both
x86_64-linux and i686-linux, ok for trunk?

2019-09-02  Jakub Jelinek  <jakub@redhat.com>

	PR go/91617
	* fold-const.c (range_check_type): For enumeral and boolean
	type, pass 1 to type_for_size langhook instead of
	TYPE_UNSIGNED (etype).  Return unsigned_type_for result whenever
	etype isn't TYPE_UNSIGNED INTEGER_TYPE.
	(build_range_check): Don't call unsigned_type_for for pointer types.
	* match.pd (X / C1 op C2): Don't call unsigned_type_for on
	range_check_type result.

--- gcc/fold-const.c.jj	2019-08-27 22:52:24.207334541 +0200
+++ gcc/fold-const.c	2019-09-01 22:46:17.091058145 +0200
@@ -4938,10 +4938,9 @@ range_check_type (tree etype)
   /* First make sure that arithmetics in this type is valid, then make sure
      that it wraps around.  */
   if (TREE_CODE (etype) == ENUMERAL_TYPE || TREE_CODE (etype) == BOOLEAN_TYPE)
-    etype = lang_hooks.types.type_for_size (TYPE_PRECISION (etype),
-					    TYPE_UNSIGNED (etype));
+    etype = lang_hooks.types.type_for_size (TYPE_PRECISION (etype), 1);
 
-  if (TREE_CODE (etype) == INTEGER_TYPE && !TYPE_OVERFLOW_WRAPS (etype))
+  if (TREE_CODE (etype) == INTEGER_TYPE && !TYPE_UNSIGNED (etype))
     {
       tree utype, minv, maxv;
 
@@ -4959,6 +4958,8 @@ range_check_type (tree etype)
       else
 	return NULL_TREE;
     }
+  else if (POINTER_TYPE_P (etype))
+    etype = unsigned_type_for (etype);
   return etype;
 }
 
@@ -5049,9 +5050,6 @@ build_range_check (location_t loc, tree
   if (etype == NULL_TREE)
     return NULL_TREE;
 
-  if (POINTER_TYPE_P (etype))
-    etype = unsigned_type_for (etype);
-
   high = fold_convert_loc (loc, etype, high);
   low = fold_convert_loc (loc, etype, low);
   exp = fold_convert_loc (loc, etype, exp);
--- gcc/match.pd.jj	2019-08-27 12:26:40.745863588 +0200
+++ gcc/match.pd	2019-09-01 18:23:02.098729356 +0200
@@ -1569,8 +1569,6 @@ (define_operator_list COND_TERNARY
 	tree etype = range_check_type (TREE_TYPE (@0));
 	if (etype)
 	  {
-	    if (! TYPE_UNSIGNED (etype))
-	      etype = unsigned_type_for (etype);
 	    hi = fold_convert (etype, hi);
 	    lo = fold_convert (etype, lo);
 	    hi = const_binop (MINUS_EXPR, etype, hi, lo);


	Jakub

Re: [PATCH] Fix up go regressions caused by my recent switchconv changes (PR go/91617)

[Andrew Pinski]

On Mon, Sep 2, 2019 at 1:14 AM Jakub Jelinek <jakub@redhat.com> wrote:
>
> On Sun, Sep 01, 2019 at 06:44:15PM +0200, Richard Biener wrote:
> > On September 1, 2019 6:34:25 PM GMT+02:00, Jakub Jelinek <jakub@redhat.com> wrote:
> > >On Sat, Aug 31, 2019 at 08:25:49PM +0200, Richard Biener wrote:
> > >> So why not always return an unsigned type then by telling
> > >type_for_size?
> > >
> > >So like this (if it passes bootstrap/regtest)?
> >
> > Yes.
>
> Unfortunately that didn't work, because TYPE_MAX_VALUE/TYPE_MIN_VALUE
> are not present on POINTER_TYPEs.
>
> Here is an updated version that passed bootstrap/regtest on both
> x86_64-linux and i686-linux, ok for trunk?

Seems like this would fix PR91632 also.
Which has a C testcase included.

Thanks,
Andrew Pinski

>
> 2019-09-02  Jakub Jelinek  <jakub@redhat.com>
>
>         PR go/91617
>         * fold-const.c (range_check_type): For enumeral and boolean
>         type, pass 1 to type_for_size langhook instead of
>         TYPE_UNSIGNED (etype).  Return unsigned_type_for result whenever
>         etype isn't TYPE_UNSIGNED INTEGER_TYPE.
>         (build_range_check): Don't call unsigned_type_for for pointer types.
>         * match.pd (X / C1 op C2): Don't call unsigned_type_for on
>         range_check_type result.
>
> --- gcc/fold-const.c.jj 2019-08-27 22:52:24.207334541 +0200
> +++ gcc/fold-const.c    2019-09-01 22:46:17.091058145 +0200
> @@ -4938,10 +4938,9 @@ range_check_type (tree etype)
>    /* First make sure that arithmetics in this type is valid, then make sure
>       that it wraps around.  */
>    if (TREE_CODE (etype) == ENUMERAL_TYPE || TREE_CODE (etype) == BOOLEAN_TYPE)
> -    etype = lang_hooks.types.type_for_size (TYPE_PRECISION (etype),
> -                                           TYPE_UNSIGNED (etype));
> +    etype = lang_hooks.types.type_for_size (TYPE_PRECISION (etype), 1);
>
> -  if (TREE_CODE (etype) == INTEGER_TYPE && !TYPE_OVERFLOW_WRAPS (etype))
> +  if (TREE_CODE (etype) == INTEGER_TYPE && !TYPE_UNSIGNED (etype))
>      {
>        tree utype, minv, maxv;
>
> @@ -4959,6 +4958,8 @@ range_check_type (tree etype)
>        else
>         return NULL_TREE;
>      }
> +  else if (POINTER_TYPE_P (etype))
> +    etype = unsigned_type_for (etype);
>    return etype;
>  }
>
> @@ -5049,9 +5050,6 @@ build_range_check (location_t loc, tree
>    if (etype == NULL_TREE)
>      return NULL_TREE;
>
> -  if (POINTER_TYPE_P (etype))
> -    etype = unsigned_type_for (etype);
> -
>    high = fold_convert_loc (loc, etype, high);
>    low = fold_convert_loc (loc, etype, low);
>    exp = fold_convert_loc (loc, etype, exp);
> --- gcc/match.pd.jj     2019-08-27 12:26:40.745863588 +0200
> +++ gcc/match.pd        2019-09-01 18:23:02.098729356 +0200
> @@ -1569,8 +1569,6 @@ (define_operator_list COND_TERNARY
>         tree etype = range_check_type (TREE_TYPE (@0));
>         if (etype)
>           {
> -           if (! TYPE_UNSIGNED (etype))
> -             etype = unsigned_type_for (etype);
>             hi = fold_convert (etype, hi);
>             lo = fold_convert (etype, lo);
>             hi = const_binop (MINUS_EXPR, etype, hi, lo);
>
>
>         Jakub

Re: [PATCH] Fix up go regressions caused by my recent switchconv changes (PR go/91617)

[Richard Biener]

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

On Mon, 2 Sep 2019, Jakub Jelinek wrote:

> On Sun, Sep 01, 2019 at 06:44:15PM +0200, Richard Biener wrote:
> > On September 1, 2019 6:34:25 PM GMT+02:00, Jakub Jelinek <jakub@redhat.com> wrote:
> > >On Sat, Aug 31, 2019 at 08:25:49PM +0200, Richard Biener wrote:
> > >> So why not always return an unsigned type then by telling
> > >type_for_size? 
> > >
> > >So like this (if it passes bootstrap/regtest)?
> > 
> > Yes. 
> 
> Unfortunately that didn't work, because TYPE_MAX_VALUE/TYPE_MIN_VALUE
> are not present on POINTER_TYPEs.
> 
> Here is an updated version that passed bootstrap/regtest on both
> x86_64-linux and i686-linux, ok for trunk?

OK.

Richard.

> 2019-09-02  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR go/91617
> 	* fold-const.c (range_check_type): For enumeral and boolean
> 	type, pass 1 to type_for_size langhook instead of
> 	TYPE_UNSIGNED (etype).  Return unsigned_type_for result whenever
> 	etype isn't TYPE_UNSIGNED INTEGER_TYPE.
> 	(build_range_check): Don't call unsigned_type_for for pointer types.
> 	* match.pd (X / C1 op C2): Don't call unsigned_type_for on
> 	range_check_type result.
> 
> --- gcc/fold-const.c.jj	2019-08-27 22:52:24.207334541 +0200
> +++ gcc/fold-const.c	2019-09-01 22:46:17.091058145 +0200
> @@ -4938,10 +4938,9 @@ range_check_type (tree etype)
>    /* First make sure that arithmetics in this type is valid, then make sure
>       that it wraps around.  */
>    if (TREE_CODE (etype) == ENUMERAL_TYPE || TREE_CODE (etype) == BOOLEAN_TYPE)
> -    etype = lang_hooks.types.type_for_size (TYPE_PRECISION (etype),
> -					    TYPE_UNSIGNED (etype));
> +    etype = lang_hooks.types.type_for_size (TYPE_PRECISION (etype), 1);
>  
> -  if (TREE_CODE (etype) == INTEGER_TYPE && !TYPE_OVERFLOW_WRAPS (etype))
> +  if (TREE_CODE (etype) == INTEGER_TYPE && !TYPE_UNSIGNED (etype))
>      {
>        tree utype, minv, maxv;
>  
> @@ -4959,6 +4958,8 @@ range_check_type (tree etype)
>        else
>  	return NULL_TREE;
>      }
> +  else if (POINTER_TYPE_P (etype))
> +    etype = unsigned_type_for (etype);
>    return etype;
>  }
>  
> @@ -5049,9 +5050,6 @@ build_range_check (location_t loc, tree
>    if (etype == NULL_TREE)
>      return NULL_TREE;
>  
> -  if (POINTER_TYPE_P (etype))
> -    etype = unsigned_type_for (etype);
> -
>    high = fold_convert_loc (loc, etype, high);
>    low = fold_convert_loc (loc, etype, low);
>    exp = fold_convert_loc (loc, etype, exp);
> --- gcc/match.pd.jj	2019-08-27 12:26:40.745863588 +0200
> +++ gcc/match.pd	2019-09-01 18:23:02.098729356 +0200
> @@ -1569,8 +1569,6 @@ (define_operator_list COND_TERNARY
>  	tree etype = range_check_type (TREE_TYPE (@0));
>  	if (etype)
>  	  {
> -	    if (! TYPE_UNSIGNED (etype))
> -	      etype = unsigned_type_for (etype);
>  	    hi = fold_convert (etype, hi);
>  	    lo = fold_convert (etype, lo);
>  	    hi = const_binop (MINUS_EXPR, etype, hi, lo);
> 
> 
> 	Jakub
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
Germany; GF: Felix Imendörffer; HRB 247165 (AG MÌnchen)

Re: [PATCH] Fix up go regressions caused by my recent switchconv changes (PR go/91617)

[Jakub Jelinek]

On Mon, Sep 02, 2019 at 01:29:24AM -0700, Andrew Pinski wrote:
> Seems like this would fix PR91632 also.
> Which has a C testcase included.

Indeed, I've committed the following after testing it with the
patch reverted as well as with current trunk where it doesn't FAIL anymore.

2019-09-02  Jakub Jelinek  <jakub@redhat.com>

	PR tree-optimization/91632
	* gcc.c-torture/execute/pr91632.c: New test.

--- gcc/testsuite/gcc.c-torture/execute/pr91632.c.jj	2019-09-02 15:28:10.598774511 +0200
+++ gcc/testsuite/gcc.c-torture/execute/pr91632.c	2019-09-02 15:28:00.540925398 +0200
@@ -0,0 +1,30 @@
+/* PR tree-optimization/91632 */
+/* { dg-additional-options "-fwrapv" } */
+
+static int
+__attribute__((noipa))
+foo (char x)
+{
+  switch (x)
+    {
+    case '"':
+    case '<':
+    case '>':
+    case '\\':
+    case '^':
+    case '`':
+    case '{':
+    case '|':
+    case '}':
+      return 0;
+    }
+  return 1;
+}
+
+int
+main ()
+{
+  if (foo ('h') == 0)
+    __builtin_abort ();
+  return 0;
+}


	Jakub