RFA: fix PR66726 regression for min/max/conversion combo from PR71016 patch

[Joern Wolfgang Rennecke]

[Apologies if this is a duplicate, I'm unsure if my previous mail was 
delivered]
On 01/07/19 12:38, Richard Biener wrote:
> On Mon, Jul 1, 2019 at 1:22 PM Joern Wolfgang Rennecke
> <joern.rennecke@riscy-ip.com> wrote:
>> The heuristic introduced for PR71016 prevents recognizing a max / min
>> combo like it is used for
>> saturation when followed by a conversion.
>> The attached patch refines the heuristic to allow this case. Regression
>> tested on x86_64-pc-linux-gnu .
> Few style nits:
>
>    ...
>
> also please check that 'lhs' is equal to gimple_assing_rhs1 (arg0_def_stmt)
> otherwise you'd also allow MIN/MAX unrelated to the conversion detected.

Like the attached patch?
>
> On x86 I see the MIN_EXPR is already detected by GENERIC folding,
> I wonder if that is required or if we can handle the case without in one
> phiopt pass invocation as well.
>
tree_ssa_phiopt_worker is supposed to handle this by handling nested 
COND_EXPRs
from the inside out:

    /* Search every basic block for COND_EXPR we may be able to optimize.

      We walk the blocks in order that guarantees that a block with
      a single predecessor is processed before the predecessor.
      This ensures that we collapse inner ifs before visiting the
      outer ones, and also that we do not try to visit a removed
      block.  */
   bb_order = single_pred_before_succ_order ();

However, I have no idea how to construct a testcase for this with the 
gimple folding in place.

#define SAT(x) (x < 0 ? 0 : (x > 255 ? 255 : x))

void
foo (unsigned char *p, int i, int m)
{
   *p = (m > SAT (i) ? m : SAT (i));
}

or the equivalent for MIN_EXPR, *.c.004.original already has only one 
conditional left.


-------------- next part --------------
2019-07-01  Joern Rennecke  <joern.rennecke@riscy-ip.com>

	PR middle-end/66726
	* tree-ssa-phiopt.c (factor_out_conditional_conversion):
	Tune heuristic from PR71016 to allow MIN / MAX.
	* testsuite/gcc.dg/tree-ssa/pr66726-4.c: New testcase.

Index: tree-ssa-phiopt.c
===================================================================
--- tree-ssa-phiopt.c	(revision 272846)
+++ tree-ssa-phiopt.c	(working copy)
@@ -504,7 +504,25 @@ factor_out_conditional_conversion (edge
 		  gsi = gsi_for_stmt (arg0_def_stmt);
 		  gsi_prev_nondebug (&gsi);
 		  if (!gsi_end_p (gsi))
-		    return NULL;
+		    {
+		      if (gassign *assign
+			    = dyn_cast <gassign *> (gsi_stmt (gsi)))
+			{
+			  tree lhs = gimple_assign_lhs (assign);
+			  enum tree_code ass_code
+			    = gimple_assign_rhs_code (assign);
+			  if (ass_code != MAX_EXPR && ass_code != MIN_EXPR)
+			    return NULL;
+			  if (!operand_equal_p
+				 (lhs, gimple_assign_rhs1 (arg0_def_stmt), 0))
+			    return NULL;
+			  gsi_prev_nondebug (&gsi);
+			  if (!gsi_end_p (gsi))
+			    return NULL;
+			}
+		      else
+			return NULL;
+		    }
 		  gsi = gsi_for_stmt (arg0_def_stmt);
 		  gsi_next_nondebug (&gsi);
 		  if (!gsi_end_p (gsi))
Index: testsuite/gcc.dg/tree-ssa/pr66726-4.c
===================================================================
--- testsuite/gcc.dg/tree-ssa/pr66726-4.c	(nonexistent)
+++ testsuite/gcc.dg/tree-ssa/pr66726-4.c	(working copy)
@@ -0,0 +1,12 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -fdump-tree-phiopt1-details" } */
+
+#define SAT(x) (x < 0 ? 0 : (x > 255 ? 255 : x))
+
+void
+foo (unsigned char *p, int i)
+{
+  *p = SAT (i);
+}
+
+/* { dg-final { scan-tree-dump-times "COND_EXPR .*and PHI .*converted to straightline code" 1 "phiopt1" } } */