public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Jakub Jelinek <jakub@redhat.com>
To: Vladimir Makarov <vmakarov@redhat.com>
Cc: gcc-patches@gcc.gnu.org
Subject: [PATCH] Fix LRA ICE in lra_substitute_pseudo on DEBUG_INSN (PR rtl-optimization/83723)
Date: Thu, 15 Feb 2018 00:49:00 -0000	[thread overview]
Message-ID: <20180214210144.GD5867@tucnak> (raw)

Hi!

Unlike normal insns where SUBREGs must properly validate, in
DEBUG_INSNs we allow arbitrary SUBREGs, either the dwarf2out code
will be able to use it, or it will just punt.  The reason for it is
among other things that during analysis we usually need to ignore
debug insns, so can't reject some optimization just because it would
create subreg in debug insn that doesn't validate, and resetting such
debug insns is too big hammer.

On the following testcase on i?86 we ICE because we have a SFmode
pseudo and want to use a XFmode new_reg for it and such subreg doesn't
validate on i386.

Fixed by using gen_rtx_raw_SUBREG in DEBUG_INSNs as other passes do.
We don't have gen_lowpart_raw_SUBREG, so the patch inlines what
gen_lowpart_SUBREG does to compute the offset and uses gen_rtx_{,raw_}SUBREG
in all cases.  Bootstrapped/regtested on x86_64-linux and i686-linux, ok for
trunk?

2018-02-14  Jakub Jelinek  <jakub@redhat.com>

	PR rtl-optimization/83723
	* lra-int.h (lra_substitute_pseudo): Add DEBUG_P argument.
	* lra.c (lra_substitute_pseudo): Likewise.  If true, use
	gen_rtx_raw_SUBREG instead of gen_rtx_SUBREG.  Pass DEBUG_P to
	recursive calls.
	(lra_substitute_pseudo_within_insn): Adjust lra_substitute_pseudo
	callers.
	* lra-constraints.c (inherit_reload_reg, split_reg): Likewise.

	* gcc.dg/pr83723.c: New test.

--- gcc/lra-int.h.jj	2018-01-03 10:19:53.848533738 +0100
+++ gcc/lra-int.h	2018-02-14 10:48:21.246324445 +0100
@@ -309,7 +309,7 @@ extern void lra_update_dups (lra_insn_re
 extern void lra_process_new_insns (rtx_insn *, rtx_insn *, rtx_insn *,
 				   const char *);
 
-extern bool lra_substitute_pseudo (rtx *, int, rtx, bool);
+extern bool lra_substitute_pseudo (rtx *, int, rtx, bool, bool);
 extern bool lra_substitute_pseudo_within_insn (rtx_insn *, int, rtx, bool);
 
 extern lra_insn_recog_data_t lra_set_insn_recog_data (rtx_insn *);
--- gcc/lra.c.jj	2018-01-03 10:19:54.726533889 +0100
+++ gcc/lra.c	2018-02-14 10:47:52.033315369 +0100
@@ -1893,9 +1893,11 @@ lra_process_new_insns (rtx_insn *insn, r
 
 /* Replace all references to register OLD_REGNO in *LOC with pseudo
    register NEW_REG.  Try to simplify subreg of constant if SUBREG_P.
-   Return true if any change was made.  */
+   DEBUG_P is if LOC is within a DEBUG_INSN.  Return true if any
+   change was made.  */
 bool
-lra_substitute_pseudo (rtx *loc, int old_regno, rtx new_reg, bool subreg_p)
+lra_substitute_pseudo (rtx *loc, int old_regno, rtx new_reg, bool subreg_p,
+		       bool debug_p)
 {
   rtx x = *loc;
   bool result = false;
@@ -1931,11 +1933,14 @@ lra_substitute_pseudo (rtx *loc, int old
       if (mode != inner_mode
 	  && ! (CONST_INT_P (new_reg) && SCALAR_INT_MODE_P (mode)))
 	{
-	  if (!partial_subreg_p (mode, inner_mode)
-	      || ! SCALAR_INT_MODE_P (inner_mode))
-	    new_reg = gen_rtx_SUBREG (mode, new_reg, 0);
+	  poly_uint64 offset = 0;
+	  if (partial_subreg_p (mode, inner_mode)
+	      && SCALAR_INT_MODE_P (inner_mode))
+	    offset = subreg_lowpart_offset (mode, inner_mode);
+	  if (debug_p)
+	    new_reg = gen_rtx_raw_SUBREG (mode, new_reg, offset);
 	  else
-	    new_reg = gen_lowpart_SUBREG (mode, new_reg);
+	    new_reg = gen_rtx_SUBREG (mode, new_reg, offset);
 	}
       *loc = new_reg;
       return true;
@@ -1948,14 +1953,14 @@ lra_substitute_pseudo (rtx *loc, int old
       if (fmt[i] == 'e')
 	{
 	  if (lra_substitute_pseudo (&XEXP (x, i), old_regno,
-				     new_reg, subreg_p))
+				     new_reg, subreg_p, debug_p))
 	    result = true;
 	}
       else if (fmt[i] == 'E')
 	{
 	  for (j = XVECLEN (x, i) - 1; j >= 0; j--)
 	    if (lra_substitute_pseudo (&XVECEXP (x, i, j), old_regno,
-				       new_reg, subreg_p))
+				       new_reg, subreg_p, debug_p))
 	      result = true;
 	}
     }
@@ -1970,7 +1975,8 @@ lra_substitute_pseudo_within_insn (rtx_i
 				   rtx new_reg, bool subreg_p)
 {
   rtx loc = insn;
-  return lra_substitute_pseudo (&loc, old_regno, new_reg, subreg_p);
+  return lra_substitute_pseudo (&loc, old_regno, new_reg, subreg_p,
+				DEBUG_INSN_P (insn));
 }
 
 \f
--- gcc/lra-constraints.c.jj	2018-02-09 06:44:36.392805568 +0100
+++ gcc/lra-constraints.c	2018-02-14 10:49:22.193340178 +0100
@@ -5287,7 +5287,8 @@ inherit_reload_reg (bool def_p, int orig
 	  lra_assert (DEBUG_INSN_P (usage_insn));
 	  next_usage_insns = XEXP (next_usage_insns, 1);
 	}
-      lra_substitute_pseudo (&usage_insn, original_regno, new_reg, false);
+      lra_substitute_pseudo (&usage_insn, original_regno, new_reg, false,
+			     DEBUG_INSN_P (usage_insn));
       lra_update_insn_regno_info (as_a <rtx_insn *> (usage_insn));
       if (lra_dump_file != NULL)
 	{
@@ -5608,7 +5609,8 @@ split_reg (bool before_p, int original_r
       usage_insn = XEXP (next_usage_insns, 0);
       lra_assert (DEBUG_INSN_P (usage_insn));
       next_usage_insns = XEXP (next_usage_insns, 1);
-      lra_substitute_pseudo (&usage_insn, original_regno, new_reg, false);
+      lra_substitute_pseudo (&usage_insn, original_regno, new_reg, false,
+			     true);
       lra_update_insn_regno_info (as_a <rtx_insn *> (usage_insn));
       if (lra_dump_file != NULL)
 	{
--- gcc/testsuite/gcc.dg/pr83723.c.jj	2018-02-14 10:58:46.269441787 +0100
+++ gcc/testsuite/gcc.dg/pr83723.c	2018-02-14 11:00:07.780456471 +0100
@@ -0,0 +1,20 @@
+/* PR rtl-optimization/83723 */
+/* { dg-do compile } */
+/* { dg-options "-g -O2" } */
+/* { dg-additional-options "-mfpmath=sse -msse2" { target i?86-*-* x86_64-*-* } } */
+/* { dg-additional-options "-fpie" { target pie } } */
+
+int foo (void);
+float bar (float);
+int *v;
+
+void
+baz (void)
+{
+  float a = bar (0.0);
+  bar (a);
+  if (v)
+    bar (1.0);
+  if (a < 1.0)
+    a = foo () / a;
+}

	Jakub

             reply	other threads:[~2018-02-15  0:49 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-02-15  0:49 Jakub Jelinek [this message]
2018-02-15 23:33 ` Vladimir Makarov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180214210144.GD5867@tucnak \
    --to=jakub@redhat.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=vmakarov@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).