Re: RFC: Add TARGET_EXPAND_COMPOUND_OPERATION

[Eric Botcazou <ebotcazou@adacore.com>]

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

> However, combiner never exposes this to x86 backend.  I added
> a TARGET_EXPAND_COMPOUND_OPERATION hook to allow x86 backend to
> optimize it.  For
>
> ---
> typedef struct
> {
>   unsigned char c1;
>   unsigned char c2;
>   unsigned char c3;
>   unsigned char c4;
> } foo_t;
>
> int
> foo (foo_t x)
> {
>    return x.c2 > 4;
> }

I think that disabling the canonicalization done by expand_compound_operation 
can disable certain forms of combining that are beneficial to x86 even for 
this kind of patterns.

The combiner already knows that it needs to re-create the compound forms when 
it is trying to simplify a comparison, see simplify_comparison.  The problem 
with your testcase is that make_compound_operation fails to do so.

Lightly tested patch attached.


	* combine.c (make_compound_operation) <SUBREG>: Do not return the
	result of force_to_mode if it partially re-expanded the compound.


-- 
Eric Botcazou

[-- Attachment #2: p.diff --]
[-- Type: text/x-diff, Size: 2228 bytes --]

Index: combine.c
===================================================================
--- combine.c	(revision 161470)
+++ combine.c	(working copy)
@@ -7277,22 +7277,21 @@ make_compound_operation (rtx x, enum rtx
       /* Call ourselves recursively on the inner expression.  If we are
 	 narrowing the object and it has a different RTL code from
 	 what it originally did, do this SUBREG as a force_to_mode.  */
-
-      tem = make_compound_operation (SUBREG_REG (x), in_code);
-
       {
-	rtx simplified = simplify_subreg (mode, tem, GET_MODE (SUBREG_REG (x)),
-					  SUBREG_BYTE (x));
+	rtx inner = SUBREG_REG (x), simplified;
+	
+	tem = make_compound_operation (inner, in_code);
 
+	simplified
+	  = simplify_subreg (mode, tem, GET_MODE (inner), SUBREG_BYTE (x));
 	if (simplified)
 	  tem = simplified;
 
-	if (GET_CODE (tem) != GET_CODE (SUBREG_REG (x))
-	    && GET_MODE_SIZE (mode) < GET_MODE_SIZE (GET_MODE (SUBREG_REG (x)))
+	if (GET_CODE (tem) != GET_CODE (inner)
+	    && GET_MODE_SIZE (mode) < GET_MODE_SIZE (GET_MODE (inner))
 	    && subreg_lowpart_p (x))
 	  {
-	    rtx newer = force_to_mode (tem, mode, ~(HOST_WIDE_INT) 0,
-				       0);
+	    rtx newer = force_to_mode (tem, mode, ~(HOST_WIDE_INT) 0, 0);
 
 	    /* If we have something other than a SUBREG, we might have
 	       done an expansion, so rerun ourselves.  */
@@ -7300,9 +7299,16 @@ make_compound_operation (rtx x, enum rtx
 	      newer = make_compound_operation (newer, in_code);
 
 	    /* force_to_mode can expand compounds.  If it just re-expanded the
-	       compound use gen_lowpart instead to convert to the desired
-	       mode.  */
-	    if (rtx_equal_p (newer, x))
+	       compound, use gen_lowpart to convert to the desired mode.  */
+	    if (rtx_equal_p (newer, x)
+		/* Likewise if it re-expanded the compound only partially.
+		   This happens for SUBREG of ZERO_EXTRACT if they extract
+		   the same number of bits.  */
+		|| (GET_CODE (newer) == SUBREG
+		    && (GET_CODE (SUBREG_REG (newer)) == LSHIFTRT
+			|| GET_CODE (SUBREG_REG (newer)) == ASHIFTRT)
+		    && GET_CODE (inner) == AND
+		    && rtx_equal_p (SUBREG_REG (newer), XEXP (inner, 0))))
 	      return gen_lowpart (GET_MODE (x), tem);
 
 	    return newer;