diff mbox

Reduce conservativeness in REE using machine model (issue6631066)

Message ID 20121010212544.33D2A61472@tjsboxrox.mtv.corp.google.com
State New
Headers show

Commit Message

Teresa Johnson Oct. 10, 2012, 9:25 p.m. UTC 
This patch addresses conservative behavior in redundant extend
elimination that was resulting in redundant extends not being
removed.

One of the checks is to ensure that the reaching definition doesn't
feed another extension with a different machine mode.

In this case, the extend we are trying to eliminate is a zero_extendqidi2,
and the other extend the reaching def is feeding is a zero_extendqisi2.
While these appear to be different because the dest modes are different
(DI vs SI), in reality zero_extendqidi2 in the machine model has an SI
attribute mode, which means that it does ultimately target an SI register:

(define_insn "zero_extend<mode>di2"
  [(set (match_operand:DI 0 "register_operand" "=r")
        (zero_extend:DI
         (match_operand:SWI12 1 "nonimmediate_operand" "<r>m")))]
  "TARGET_64BIT"
  "movz{<imodesuffix>l|x}\t{%1, %k0|%k0, %1}"
  [(set_attr "type" "imovx")
   (set_attr "mode" "SI")])

What I did to address this is to call get_attr_mode from the machine model
to get the actual mode of the insn. In this case, it returns MODE_SI.
There doesn't seem to be any code that maps from the attr_mode (MODE_SI)
to the machine_mode (SImode), so I am doing the mapping in ree.c before
the comparison with the mode from the second extend.

Bootstrapped and tested on x86_64-unknown-linux-gnu. Is this ok for trunk?

Thanks,
Teresa

2012-10-10  Teresa Johnson  <tejohnson@google.com>

	* ree.c (get_mode): New function.
	(add_removable_extension): Use get_mode to obtain the
        machine mode for comparison with other extends.


--
This patch is available for review at http://codereview.appspot.com/6631066

Comments

Steven Bosscher Oct. 10, 2012, 9:43 p.m. UTC | #1 
On Wed, Oct 10, 2012 at 11:25 PM, Teresa Johnson <tejohnson@google.com> wrote:
> What I did to address this is to call get_attr_mode from the machine model
> to get the actual mode of the insn. In this case, it returns MODE_SI.
> There doesn't seem to be any code that maps from the attr_mode (MODE_SI)
> to the machine_mode (SImode), so I am doing the mapping in ree.c before
> the comparison with the mode from the second extend.

The "mode" attribute is not a default attribute, i.e. targets are free
to define an attribute mode with target-specific semantics. So AFAIU,
I don't think you can just use the i386 "mode" attribute in ree.c.

So maybe you need a target hook, something similar to mode_rep_extended.

Ciao!
Steven
diff mbox

Patch

Index: ree.c
===================================================================
--- ree.c	(revision 192265)
+++ ree.c	(working copy)
@@ -240,6 +240,7 @@  along with GCC; see the file COPYING3.  If not see
 #include "tree-pass.h"
 #include "df.h"
 #include "cgraph.h"
+#include "insn-attr.h"
 
 /* This structure represents a candidate for elimination.  */
 
@@ -756,6 +757,41 @@  combine_reaching_defs (ext_cand *cand, const_rtx s
   return false;
 }
 
+/* Given an INSN, obtain the attr_mode specified by the machine
+   model, and map it to the corresponding machine_mode. If the
+   attr_mode isn't available, return the machine mode for DEST.  */
+
+static enum machine_mode
+get_mode (rtx insn, rtx dest)
+{
+  enum machine_mode mode;
+
+#ifdef HAVE_ATTR_mode
+  switch (get_attr_mode (insn))
+    {
+      case MODE_QI:
+        mode = QImode;
+        break;
+      case MODE_HI:
+        mode = HImode;
+        break;
+      case MODE_SI:
+        mode = SImode;
+        break;
+      case MODE_DI:
+        mode = DImode;
+        break;
+      default:
+        mode = GET_MODE (dest);
+        break;
+    }
+#else
+  mode = GET_MODE (dest);
+#endif
+
+  return mode;
+}
+
 /* Add an extension pattern that could be eliminated.  */
 
 static void
@@ -775,7 +811,7 @@  add_removable_extension (const_rtx expr, rtx insn,
   src = SET_SRC (expr);
   code = GET_CODE (src);
   dest = SET_DEST (expr);
-  mode = GET_MODE (dest);
+  mode = get_mode (insn, dest);
 
   if (REG_P (dest)
       && (code == SIGN_EXTEND || code == ZERO_EXTEND)