Patchwork Fix DEBUG_INSN handling in lra-constraints (PR rtl-optimization/55193)

login
register
mail settings
Submitter Jakub Jelinek
Date Dec. 11, 2012, 8:21 a.m.
Message ID <20121211082149.GM2315@tucnak.redhat.com>
Download mbox | patch
Permalink /patch/205137/
State New
Headers show

Comments

Jakub Jelinek - Dec. 11, 2012, 8:21 a.m.
Hi!

As the testcase in the PR (unfortunately only reproduceable after reverting
some inlining patch, so not including that testcase in the patch) shows,
loc_equivalence_change_p isn't good enough for debug_insns, when a REG is
substituted for VOIDmode constant.  In that case, the surrounding SUBREG
(handled in loc_equivalence_change_p), but also other operations like
ZERO_EXTEND/SIGN_EXTEND, ZERO_EXTRACT etc. need to be simplified while we
still know the original mode, and perhaps several times, as SIGN_EXTEND
might be in another ZERO_EXTEND etc.  The best way to deal with that
is simplify_replace_fn_rtx which is used in various other places in the
compiler.

Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux, ok for
trunk?

2012-12-11  Jakub Jelinek  <jakub@redhat.com>

	PR rtl-optimization/55193
	* lra-constraints.c (loc_equivalence_callback): New function.
	(lra_constraints): Call simplify_replace_fn_rtx instead of
	loc_equivalence_change_p on DEBUG_INSNs.


	Jakub
Vladimir Makarov - Dec. 11, 2012, 5:23 p.m.
On 12/11/2012 03:21 AM, Jakub Jelinek wrote:
> Hi!
>
> As the testcase in the PR (unfortunately only reproduceable after reverting
> some inlining patch, so not including that testcase in the patch) shows,
> loc_equivalence_change_p isn't good enough for debug_insns, when a REG is
> substituted for VOIDmode constant.  In that case, the surrounding SUBREG
> (handled in loc_equivalence_change_p), but also other operations like
> ZERO_EXTEND/SIGN_EXTEND, ZERO_EXTRACT etc. need to be simplified while we
> still know the original mode, and perhaps several times, as SIGN_EXTEND
> might be in another ZERO_EXTEND etc.  The best way to deal with that
> is simplify_replace_fn_rtx which is used in various other places in the
> compiler.
>
> Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux, ok for
> trunk?
Yes.  Thanks for working on it, Jakub.
> 2012-12-11  Jakub Jelinek  <jakub@redhat.com>
>
> 	PR rtl-optimization/55193
> 	* lra-constraints.c (loc_equivalence_callback): New function.
> 	(lra_constraints): Call simplify_replace_fn_rtx instead of
> 	loc_equivalence_change_p on DEBUG_INSNs.
>
>

Patch

--- gcc/lra-constraints.c.jj	2012-12-10 08:42:40.000000000 +0100
+++ gcc/lra-constraints.c	2012-12-10 18:43:04.977277475 +0100
@@ -3186,6 +3186,21 @@  loc_equivalence_change_p (rtx *loc)
   return result;
 }
 
+/* Similar to loc_equivalence_change_p, but for use as
+   simplify_replace_fn_rtx callback.  */
+static rtx
+loc_equivalence_callback (rtx loc, const_rtx, void *)
+{
+  if (!REG_P (loc))
+    return NULL_RTX;
+
+  rtx subst = get_equiv_substitution (loc);
+  if (subst != loc)
+    return subst;
+
+  return NULL_RTX;
+}
+
 /* Maximum allowed number of constraint pass iterations after the last
    spill pass.	It is for preventing LRA cycling in a bug case.	 */
 #define MAX_CONSTRAINT_ITERATION_NUMBER 30
@@ -3422,11 +3437,17 @@  lra_constraints (bool first_p)
 	  /* We need to check equivalence in debug insn and change
 	     pseudo to the equivalent value if necessary.  */
 	  curr_id = lra_get_insn_recog_data (curr_insn);
-	  if (bitmap_bit_p (&equiv_insn_bitmap, INSN_UID (curr_insn))
-	      && loc_equivalence_change_p (curr_id->operand_loc[0]))
+	  if (bitmap_bit_p (&equiv_insn_bitmap, INSN_UID (curr_insn)))
 	    {
-	      lra_update_insn_regno_info (curr_insn);
-	      changed_p = true;
+	      rtx old = *curr_id->operand_loc[0];
+	      *curr_id->operand_loc[0]
+		= simplify_replace_fn_rtx (old, NULL_RTX,
+					   loc_equivalence_callback, NULL);
+	      if (old != *curr_id->operand_loc[0])
+		{
+		  lra_update_insn_regno_info (curr_insn);
+		  changed_p = true;
+		}
 	    }
 	}
       else if (INSN_P (curr_insn))