[4/n] Remove GENERIC stmt combining from SCCVN

This moves fold_sign_changed_comparison.  Shows up in gcc.dg/pr55833.c

I'll eventually massage it according to Jakubs suggestion to do a

#ifndef HAVE_canonicalize_funcptr_for_compare
#define HAVE_canonicalize_funcptr_for_compare 0
#endif

somewhere (defaults.h should work I guess).

Bootstrap and regtest running on x86_64-unknown-linux-gnu.

Richard.

gcc.dg/pr55833.c

2015-06-25  Richard Biener  <rguenther@suse.de>

	* fold-const.c (fold_sign_changed_comparison): Remove.
	* match.pd: Instead add same functionality here.

On Thu, Jun 25, 2015 at 03:24:33PM +0200, Richard Biener wrote:
> 
> This moves fold_sign_changed_comparison.  Shows up in gcc.dg/pr55833.c
> 
> I'll eventually massage it according to Jakubs suggestion to do a
> 
> #ifndef HAVE_canonicalize_funcptr_for_compare
> #define HAVE_canonicalize_funcptr_for_compare 0
> #endif
> 
> somewhere (defaults.h should work I guess).

The alternative is to switch all of the HAVE_* macros generated by
genflags.h into always defined macros as part of decreasing the amount of
#ifdefs in the source (so one liner in genflags, and then go through all the
#ifdef HAVE_* and #ifndef HAVE_* in the sources and deal with them.

	Jakub

On Thu, 25 Jun 2015, Richard Biener wrote:

> 
> This moves fold_sign_changed_comparison.  Shows up in gcc.dg/pr55833.c
> 
> I'll eventually massage it according to Jakubs suggestion to do a
> 
> #ifndef HAVE_canonicalize_funcptr_for_compare
> #define HAVE_canonicalize_funcptr_for_compare 0
> #endif
> 
> somewhere (defaults.h should work I guess).
> 
> Bootstrap and regtest running on x86_64-unknown-linux-gnu.

This runs into

Running target unix//-m32
FAIL: g++.dg/tree-ssa/calloc.C  -std=gnu++11  scan-tree-dump-not optimized 
"malloc"

where we now optimize

   n_5 = (size_t) n_4(D);
...
   <bb 5>:
   -  if (n_5 != 0)
   +  if (n_4(D) != 0)

but both VRP and DOM fail to record equivalences for n_5 from
the updated condition (I have a patch to fix VRP but not DOM).

When we restrict the transform to trigger on single-use n_5
only the patch doesn't fix the SCCVN regression that shows up
in gcc.dg/pr55833.c (SCCVN currently uses GENERIC folding which
doesn't care about single-uses and thus if switched to GIMPLE
folding it suddenly regresses).

Putting this one on hold for the moment.

I think we're simply missing a pass that can "properly" deal
with this kind of stuff.  For example DOM could re-visit
stmts which have uses of SSA names we added temporary
equivalences for (ones that dominate the current block,
others we'll (re-)visit anyway).  That would fix this testcase
but to catch secondary effects you'd need to re-visit uses of
the defs of the revisited stmts as well (if anything interesting
was produced, of course).

Richard.

> Richard.
> 
> gcc.dg/pr55833.c
> 
> 2015-06-25  Richard Biener  <rguenther@suse.de>
> 
> 	* fold-const.c (fold_sign_changed_comparison): Remove.
> 	* match.pd: Instead add same functionality here.
> 
> Index: gcc/fold-const.c
> ===================================================================
> --- gcc/fold-const.c	(revision 224893)
> +++ gcc/fold-const.c	(working copy)
> @@ -7044,57 +7044,6 @@ fold_widened_comparison (location_t loc,
>    return NULL_TREE;
>  }
>  
> -/* Fold comparison ARG0 CODE ARG1 (with result in TYPE), where for
> -   ARG0 just the signedness is changed.  */
> -
> -static tree
> -fold_sign_changed_comparison (location_t loc, enum tree_code code, tree type,
> -			      tree arg0, tree arg1)
> -{
> -  tree arg0_inner;
> -  tree inner_type, outer_type;
> -
> -  if (!CONVERT_EXPR_P (arg0))
> -    return NULL_TREE;
> -
> -  outer_type = TREE_TYPE (arg0);
> -  arg0_inner = TREE_OPERAND (arg0, 0);
> -  inner_type = TREE_TYPE (arg0_inner);
> -
> -#ifdef HAVE_canonicalize_funcptr_for_compare
> -  /* Disable this optimization if we're casting a function pointer
> -     type on targets that require function pointer canonicalization.  */
> -  if (HAVE_canonicalize_funcptr_for_compare
> -      && TREE_CODE (inner_type) == POINTER_TYPE
> -      && TREE_CODE (TREE_TYPE (inner_type)) == FUNCTION_TYPE)
> -    return NULL_TREE;
> -#endif
> -
> -  if (TYPE_PRECISION (inner_type) != TYPE_PRECISION (outer_type))
> -    return NULL_TREE;
> -
> -  if (TREE_CODE (arg1) != INTEGER_CST
> -      && !(CONVERT_EXPR_P (arg1)
> -	   && TREE_TYPE (TREE_OPERAND (arg1, 0)) == inner_type))
> -    return NULL_TREE;
> -
> -  if (TYPE_UNSIGNED (inner_type) != TYPE_UNSIGNED (outer_type)
> -      && code != NE_EXPR
> -      && code != EQ_EXPR)
> -    return NULL_TREE;
> -
> -  if (POINTER_TYPE_P (inner_type) != POINTER_TYPE_P (outer_type))
> -    return NULL_TREE;
> -
> -  if (TREE_CODE (arg1) == INTEGER_CST)
> -    arg1 = force_fit_type (inner_type, wi::to_widest (arg1), 0,
> -			   TREE_OVERFLOW (arg1));
> -  else
> -    arg1 = fold_convert_loc (loc, inner_type, arg1);
> -
> -  return fold_build2_loc (loc, code, type, arg0_inner, arg1);
> -}
> -
>  
>  /* Fold A < X && A + 1 > Y to A < X && A >= Y.  Normally A + 1 > Y
>     means A >= Y && A != MAX, but in this case we know that
> @@ -9252,11 +9201,6 @@ fold_comparison (location_t loc, enum tr
>        tem = fold_widened_comparison (loc, code, type, arg0, arg1);
>        if (tem)
>  	return tem;
> -
> -      /* Or if we are changing signedness.  */
> -      tem = fold_sign_changed_comparison (loc, code, type, arg0, arg1);
> -      if (tem)
> -	return tem;
>      }
>  
>    /* If this is comparing a constant with a MIN_EXPR or a MAX_EXPR of a
> Index: gcc/match.pd
> ===================================================================
> --- gcc/match.pd	(revision 224893)
> +++ gcc/match.pd	(working copy)
> @@ -1086,6 +1196,30 @@ (define_operator_list swapped_tcc_compar
>      (if (tem && !TREE_OVERFLOW (tem))
>       (scmp @0 { tem; }))))))
>  
> +/* From fold_sign_changed_comparison.  */
> +(for cmp (tcc_comparison)
> + (simplify
> +  (cmp (convert@0 @00) (convert?@1 @10))
> +  (if (TREE_CODE (TREE_TYPE (@0)) == INTEGER_TYPE)
> +   /* Disable this optimization if we're casting a function pointer
> +      type on targets that require function pointer canonicalization.  */
> +#ifdef HAVE_canonicalize_funcptr_for_compare
> +   (if (!(HAVE_canonicalize_funcptr_for_compare
> +	  && TREE_CODE (TREE_TYPE (@00)) == POINTER_TYPE
> +	  && TREE_CODE (TREE_TYPE (TREE_TYPE (@00))) == FUNCTION_TYPE))
> +#endif
> +    (if (TYPE_PRECISION (TREE_TYPE (@00)) == TYPE_PRECISION (TREE_TYPE (@0))
> +	 && (TREE_CODE (@10) == INTEGER_CST
> +	     || (@1 != @10 && types_match (TREE_TYPE (@10), TREE_TYPE (@00))))
> +	 && (TYPE_UNSIGNED (TREE_TYPE (@00)) == TYPE_UNSIGNED (TREE_TYPE (@0))
> +	     || cmp == NE_EXPR
> +	     || cmp == EQ_EXPR)
> +	 && (POINTER_TYPE_P (TREE_TYPE (@00)) == POINTER_TYPE_P (TREE_TYPE (@0))))
> +     (cmp @00 (convert @1))))))
> +#ifdef HAVE_canonicalize_funcptr_for_compare
> +)
> +#endif
> +
>  /* Simplification of math builtins.  */
>  
>  (define_operator_list LOG BUILT_IN_LOGF BUILT_IN_LOG BUILT_IN_LOGL)
>

On 06/26/2015 03:24 AM, Richard Biener wrote:
> On Thu, 25 Jun 2015, Richard Biener wrote:
>
>>
>> This moves fold_sign_changed_comparison.  Shows up in gcc.dg/pr55833.c
>>
>> I'll eventually massage it according to Jakubs suggestion to do a
>>
>> #ifndef HAVE_canonicalize_funcptr_for_compare
>> #define HAVE_canonicalize_funcptr_for_compare 0
>> #endif
>>
>> somewhere (defaults.h should work I guess).
>>
>> Bootstrap and regtest running on x86_64-unknown-linux-gnu.
>
> This runs into
>
> Running target unix//-m32
> FAIL: g++.dg/tree-ssa/calloc.C  -std=gnu++11  scan-tree-dump-not optimized
> "malloc"
>
> where we now optimize
>
>     n_5 = (size_t) n_4(D);
> ...
>     <bb 5>:
>     -  if (n_5 != 0)
>     +  if (n_4(D) != 0)
>
> but both VRP and DOM fail to record equivalences for n_5 from
> the updated condition (I have a patch to fix VRP but not DOM).
So you want an equivalence recorded for n_5, even though it no longer 
appears in the conditional (but presumably has other uses)?

DOM has some code for this already, but it's kind-of backwards from what 
you're trying to do in terms of when it's used.  That code might be 
factorable and usable elsewhere in DOM.




>
> I think we're simply missing a pass that can "properly" deal
> with this kind of stuff.  For example DOM could re-visit
> stmts which have uses of SSA names we added temporary
> equivalences for (ones that dominate the current block,
> others we'll (re-)visit anyway).  That would fix this testcase
> but to catch secondary effects you'd need to re-visit uses of
> the defs of the revisited stmts as well (if anything interesting
> was produced, of course).
This problem feels a bit like it's better handled by an optimizer 
independent of DOM.  Probably a backwards IL walking context sensitive 
optimizer.  I want to do something like that for threading, but haven't 
much pondered other use cases for that kind of structure.

If you could create a BZ and include the patches you're playing with and 
a reference to the failing test (calloc.C for -m32), it'd be appreciated.

Jeff