From patchwork Mon Jun 28 10:05:11 2010
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Subject: RFC: Add TARGET_EXPAND_COMPOUND_OPERATION
From: Eric Botcazou <ebotcazou@adacore.com>
X-Patchwork-Id: 57127
Message-Id: <201006281205.11210.ebotcazou@adacore.com>
To: "H.J. Lu" <hjl.tools@gmail.com>
Cc: gcc-patches@gcc.gnu.org
Date: Mon, 28 Jun 2010 12:05:11 +0200

> 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.


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;