This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[rtl-optimization/61094] Do not re-extend in ree.c if a copy was needed



As discussed in the PR, we have an extension we want to eliminate and doing so requires a copy.

We later find another extension we want to eliminate and doing so would require widening the copy, which may not result in a valid insn.

This patch does two things.

First, when we want to eliminate an extension and a copy is needed, we generate the copy, verify its a single insn, recognize it and verify it matches its constraints. If any of those fail, then we can not eliminate the extension.

Second, when we eliminate an extension using a copy, we mark it as do_not_reextend, which avoids the problems noted above. Note this is only done when we have an extension that is eliminated and a copy was needed to do so.

While this potentially disables an optimization, these situations are exceedingly rare. Obviously if someone finds code that suffers as a result, we'll have to go with a more complex solution.

Bootstrapped and regression tested on x86_64-unknown-linux-gnu. Installed on the trunk.

commit 13d0ab22e1eb98e20f63bb33ca33f2b1599b1598
Author: law <law@138bc75d-0d04-0410-961f-82ee72b054a4>
Date:   Mon Jun 2 19:12:08 2014 +0000

    	PR rtl-optimization/61094
    	* ree.c (combine_reaching_defs): Do not reextend an insn if it
    	was marked as do_no_reextend.  If a copy is needed to eliminate
    	an extension, then mark it as do_not_reextend.
    
    	PR rtl-optimization/61094
    	* g++.dg/pr61094: New test.
    
    git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/trunk@211142 138bc75d-0d04-0410-961f-82ee72b054a4

diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index cffab0b..acefcc0 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,10 @@
+2014-06-02  Jeff Law  <law@redhat.com>
+
+	PR rtl-optimization/61094
+	* ree.c (combine_reaching_defs): Do not reextend an insn if it
+	was marked as do_no_reextend.  If a copy is needed to eliminate
+	an extension, then mark it as do_not_reextend.
+
 2014-06-02  Marcus Shawcroft  <marcus.shawcroft@arm.com>
 
 	* config/aarch64/aarch64.md (set_fpcr): Drop ISB after FPCR write.
diff --git a/gcc/ree.c b/gcc/ree.c
index 77f1384..ade413e 100644
--- a/gcc/ree.c
+++ b/gcc/ree.c
@@ -507,6 +507,8 @@ struct ATTRIBUTE_PACKED ext_modified
   /* Kind of modification of the insn.  */
   ENUM_BITFIELD(ext_modified_kind) kind : 2;
 
+  unsigned int do_not_reextend : 1;
+
   /* True if the insn is scheduled to be deleted.  */
   unsigned int deleted : 1;
 };
@@ -712,8 +714,10 @@ combine_reaching_defs (ext_cand *cand, const_rtx set_pat, ext_state *state)
      register than the source operand, then additional restrictions
      are needed.  Note we have to handle cases where we have nested
      extensions in the source operand.  */
-  if (REGNO (SET_DEST (PATTERN (cand->insn)))
-      != REGNO (get_extended_src_reg (SET_SRC (PATTERN (cand->insn)))))
+  bool copy_needed
+    = (REGNO (SET_DEST (PATTERN (cand->insn)))
+       != REGNO (get_extended_src_reg (SET_SRC (PATTERN (cand->insn)))));
+  if (copy_needed)
     {
       /* In theory we could handle more than one reaching def, it
 	 just makes the code to update the insn stream more complex.  */
@@ -722,7 +726,7 @@ combine_reaching_defs (ext_cand *cand, const_rtx set_pat, ext_state *state)
 
       /* We require the candidate not already be modified.  It may,
 	 for example have been changed from a (sign_extend (reg))
-	 into (zero_extend (sign_extend (reg)).
+	 into (zero_extend (sign_extend (reg))).
 
 	 Handling that case shouldn't be terribly difficult, but the code
 	 here and the code to emit copies would need auditing.  Until
@@ -777,6 +781,31 @@ combine_reaching_defs (ext_cand *cand, const_rtx set_pat, ext_state *state)
 	  || reg_set_between_p (SET_DEST (PATTERN (cand->insn)),
 				def_insn, cand->insn))
 	return false;
+
+      /* We must be able to copy between the two registers.   Generate,
+	 recognize and verify constraints of the copy.  Also fail if this
+	 generated more than one insn.
+
+         This generates garbage since we throw away the insn when we're
+	 done, only to recreate it later if this test was successful.  */
+      start_sequence ();
+      rtx sub_rtx = *get_sub_rtx (def_insn);
+      rtx pat = PATTERN (cand->insn);
+      rtx new_dst = gen_rtx_REG (GET_MODE (SET_DEST (sub_rtx)),
+                                 REGNO (XEXP (SET_SRC (pat), 0)));
+      rtx new_src = gen_rtx_REG (GET_MODE (SET_DEST (sub_rtx)),
+                                 REGNO (SET_DEST (pat)));
+      emit_move_insn (new_dst, new_src);
+
+      rtx insn = get_insns();
+      end_sequence ();
+      if (NEXT_INSN (insn))
+	return false;
+      if (recog_memoized (insn) == -1)
+	return false;
+      extract_insn (insn);
+      if (!constrain_operands (1))
+	return false;
     }
 
 
@@ -843,11 +872,15 @@ combine_reaching_defs (ext_cand *cand, const_rtx set_pat, ext_state *state)
             fprintf (dump_file, "All merges were successful.\n");
 
 	  FOR_EACH_VEC_ELT (state->modified_list, i, def_insn)
-	    if (state->modified[INSN_UID (def_insn)].kind == EXT_MODIFIED_NONE)
-	      state->modified[INSN_UID (def_insn)].kind
-		= (cand->code == ZERO_EXTEND
-		   ? EXT_MODIFIED_ZEXT : EXT_MODIFIED_SEXT);
+	    {
+	      ext_modified *modified = &state->modified[INSN_UID (def_insn)];
+	      if (modified->kind == EXT_MODIFIED_NONE)
+		modified->kind = (cand->code == ZERO_EXTEND ? EXT_MODIFIED_ZEXT
+						            : EXT_MODIFIED_SEXT);
 
+	      if (copy_needed)
+		modified->do_not_reextend = 1;
+	    }
           return true;
         }
       else
diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog
index a52af2b..590969e 100644
--- a/gcc/testsuite/ChangeLog
+++ b/gcc/testsuite/ChangeLog
@@ -1,3 +1,8 @@
+2014-06-02  Jeff Law  <law@redhat.com>
+
+	PR rtl-optimization/61094
+	* g++.dg/pr61094: New test.
+
 2014-06-02  Uros Bizjak  <ubizjak@gmail.com>
 
 	* gcc.target/i386/xop-rotate1-vector.c (dg-options): Add -mno-avx2.
diff --git a/gcc/testsuite/g++.dg/pr61094.C b/gcc/testsuite/g++.dg/pr61094.C
new file mode 100644
index 0000000..35adc25
--- /dev/null
+++ b/gcc/testsuite/g++.dg/pr61094.C
@@ -0,0 +1,31 @@
+/* { dg-do compile } */
+/* { dg-options "-O3" }  */
+
+template <typename> struct A {
+  unsigned _width, _height, _depth, _spectrum;
+  template <typename t> A(t p1) {
+    int a = p1.size();
+    if (a) {
+      _width = p1._width;
+      _depth = _height = _spectrum = p1._spectrum;
+    }
+  }
+  long size() { return (long)_width * _height * _depth * _spectrum; }
+};
+
+int d;
+void fn1(void *);
+A<int> *fn2();
+void fn3() {
+  int b;
+  for (;;) {
+    A<char> c(*fn2());
+    fn1(&c);
+    if (d || !b)
+      throw;
+  }
+}
+
+
+
+

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]