public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [pushed][PR112918][LRA]: Fixing IRA ICE on m68k
@ 2023-12-18 22:16 Vladimir Makarov
  2023-12-19  0:05 ` Jeff Law
  0 siblings, 1 reply; 6+ messages in thread
From: Vladimir Makarov @ 2023-12-18 22:16 UTC (permalink / raw)
  To: gcc-patches

[-- Attachment #1: Type: text/plain, Size: 341 bytes --]

The following patch fixes

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112918

The patch was successfully bootstrapped and tested on x86-64, aarch64, 
and ppc64.

The patch affects a sensitive part of LRA.  So I will monitor that the 
commit does not create serious failures on other targets. If it happens, 
I probably revert the patch.


[-- Attachment #2: pr112918.patch --]
[-- Type: text/x-patch, Size: 3918 bytes --]

commit 989e67f827b74b76e58abe137ce12d948af2290c
Author: Vladimir N. Makarov <vmakarov@redhat.com>
Date:   Mon Dec 18 17:12:23 2023 -0500

    [PR112918][LRA]: Fixing IRA ICE on m68k
    
    Some GCC tests on m68K port of LRA is failed on `maximum number of
    generated reload insns per insn achieved`.  The problem is in that for
    subreg reload LRA can not narrow reg class more from ALL_REGS to
    GENERAL_REGS and then to data regs or address regs.  The patch permits
    narowing reg class from reload insns if this results in succesful
    matching of reg operand.
    
    gcc/ChangeLog:
    
            PR rtl-optimization/112918
            * lra-constraints.cc (SMALL_REGISTER_CLASS_P): Move before in_class_p.
            (in_class_p): Restrict condition for narrowing class in case of
            allow_all_reload_class_changes_p.
            (process_alt_operands): Pass true for
            allow_all_reload_class_changes_p in calls of in_class_p.
            (curr_insn_transform): Ditto for reg operand win.

diff --git a/gcc/lra-constraints.cc b/gcc/lra-constraints.cc
index da7e1748d75..05479ab98dd 100644
--- a/gcc/lra-constraints.cc
+++ b/gcc/lra-constraints.cc
@@ -261,6 +261,13 @@ enough_allocatable_hard_regs_p (enum reg_class reg_class,
   return false;
 }
 
+/* True if C is a non-empty register class that has too few registers
+   to be safely used as a reload target class.	*/
+#define SMALL_REGISTER_CLASS_P(C)		\
+  (ira_class_hard_regs_num [(C)] == 1		\
+   || (ira_class_hard_regs_num [(C)] >= 1	\
+       && targetm.class_likely_spilled_p (C)))
+
 /* Return true if REG satisfies (or will satisfy) reg class constraint
    CL.  Use elimination first if REG is a hard register.  If REG is a
    reload pseudo created by this constraints pass, assume that it will
@@ -318,7 +325,11 @@ in_class_p (rtx reg, enum reg_class cl, enum reg_class *new_class,
       common_class = ira_reg_class_subset[rclass][cl];
       if (new_class != NULL)
 	*new_class = common_class;
-      return enough_allocatable_hard_regs_p (common_class, reg_mode);
+      return (enough_allocatable_hard_regs_p (common_class, reg_mode)
+	      /* Do not permit reload insn operand matching (new_class == NULL
+		 case) if the new class is too small.  */
+	      && (new_class != NULL || common_class == rclass
+		  || !SMALL_REGISTER_CLASS_P (common_class)));
     }
 }
 
@@ -923,13 +934,6 @@ operands_match_p (rtx x, rtx y, int y_hard_regno)
    && GET_MODE_SIZE (MODE).is_constant ()	\
    && !targetm.cannot_force_const_mem (MODE, X))
 
-/* True if C is a non-empty register class that has too few registers
-   to be safely used as a reload target class.	*/
-#define SMALL_REGISTER_CLASS_P(C)		\
-  (ira_class_hard_regs_num [(C)] == 1		\
-   || (ira_class_hard_regs_num [(C)] >= 1	\
-       && targetm.class_likely_spilled_p (C)))
-
 /* If REG is a reload pseudo, try to make its class satisfying CL.  */
 static void
 narrow_reload_pseudo_class (rtx reg, enum reg_class cl)
@@ -2631,7 +2635,7 @@ process_alt_operands (int only_alternative)
 				    hard_regno[nop]))))
 			win = true;
 		      else if (hard_regno[nop] < 0
-			       && in_class_p (op, this_alternative, NULL))
+			       && in_class_p (op, this_alternative, NULL, true))
 			win = true;
 		    }
 		  break;
@@ -2675,7 +2679,7 @@ process_alt_operands (int only_alternative)
 			  reject++;
 			}
 		      if (in_class_p (operand_reg[nop],
-				      this_costly_alternative, NULL))
+				      this_costly_alternative, NULL, true))
 			{
 			  if (lra_dump_file != NULL)
 			    fprintf
@@ -4388,7 +4392,7 @@ curr_insn_transform (bool check_only_p)
 
 	if (REG_P (reg) && (regno = REGNO (reg)) >= FIRST_PSEUDO_REGISTER)
 	  {
-	    bool ok_p = in_class_p (reg, goal_alt[i], &new_class);
+	    bool ok_p = in_class_p (reg, goal_alt[i], &new_class, true);
 
 	    if (new_class != NO_REGS && get_reg_class (regno) != new_class)
 	      {

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [pushed][PR112918][LRA]: Fixing IRA ICE on m68k
  2023-12-18 22:16 [pushed][PR112918][LRA]: Fixing IRA ICE on m68k Vladimir Makarov
@ 2023-12-19  0:05 ` Jeff Law
  2023-12-19  3:34   ` Jeff Law
  2023-12-20 20:35   ` Joseph Myers
  0 siblings, 2 replies; 6+ messages in thread
From: Jeff Law @ 2023-12-19  0:05 UTC (permalink / raw)
  To: Vladimir Makarov, gcc-patches



On 12/18/23 15:16, Vladimir Makarov wrote:
> The following patch fixes
> 
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112918
> 
> The patch was successfully bootstrapped and tested on x86-64, aarch64, 
> and ppc64.
> 
> The patch affects a sensitive part of LRA.  So I will monitor that the 
> commit does not create serious failures on other targets. If it happens, 
> I probably revert the patch.
I've also spun up my tester with this patch.  We should know if there's 
any fallout on the cross targets by tomorrow AM.  The native emulated 
targets will take longer to trickle in.

jeff

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [pushed][PR112918][LRA]: Fixing IRA ICE on m68k
  2023-12-19  0:05 ` Jeff Law
@ 2023-12-19  3:34   ` Jeff Law
  2023-12-20 20:35   ` Joseph Myers
  1 sibling, 0 replies; 6+ messages in thread
From: Jeff Law @ 2023-12-19  3:34 UTC (permalink / raw)
  To: Vladimir Makarov, gcc-patches



On 12/18/23 17:05, Jeff Law wrote:
> 
> 
> On 12/18/23 15:16, Vladimir Makarov wrote:
>> The following patch fixes
>>
>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112918
>>
>> The patch was successfully bootstrapped and tested on x86-64, aarch64, 
>> and ppc64.
>>
>> The patch affects a sensitive part of LRA.  So I will monitor that the 
>> commit does not create serious failures on other targets. If it 
>> happens, I probably revert the patch.
> I've also spun up my tester with this patch.  We should know if there's 
> any fallout on the cross targets by tomorrow AM.  The native emulated 
> targets will take longer to trickle in.
Crosses done.  No changes in test output.  It's testing hppa and sh4 
natives with qemu emulation right now.  Those should land in ~12 hrs. 
And there'll be 1-2 landing daily through the week.

jeff

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [pushed][PR112918][LRA]: Fixing IRA ICE on m68k
  2023-12-19  0:05 ` Jeff Law
  2023-12-19  3:34   ` Jeff Law
@ 2023-12-20 20:35   ` Joseph Myers
  1 sibling, 0 replies; 6+ messages in thread
From: Joseph Myers @ 2023-12-20 20:35 UTC (permalink / raw)
  To: Jeff Law; +Cc: Vladimir Makarov, gcc-patches

[-- Attachment #1: Type: text/plain, Size: 1190 bytes --]

On Mon, 18 Dec 2023, Jeff Law wrote:

> 
> 
> On 12/18/23 15:16, Vladimir Makarov wrote:
> > The following patch fixes
> > 
> > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112918
> > 
> > The patch was successfully bootstrapped and tested on x86-64, aarch64, and
> > ppc64.
> > 
> > The patch affects a sensitive part of LRA.  So I will monitor that the
> > commit does not create serious failures on other targets. If it happens, I
> > probably revert the patch.
> I've also spun up my tester with this patch.  We should know if there's any
> fallout on the cross targets by tomorrow AM.  The native emulated targets will
> take longer to trickle in.

My glibc bot shows ICEs for arc and mips (different ICEs, so I filed two 
separate bugs).

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113097
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113098
https://sourceware.org/pipermail/libc-testresults/2023q4/012248.html

(The reason the bot only shows the mips ICE in nan2008 configurations is 
probably that those use --with-arch-32=mips32r2 and the other 
configurations don't, rather than any actual connection to the nan2008 
feature itself.)

-- 
Joseph S. Myers
joseph@codesourcery.com

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [pushed][PR112918][LRA]: Fixing IRA ICE on m68k
  2024-01-11 14:35 Vladimir Makarov
@ 2024-01-12 14:33 ` YunQiang Su
  0 siblings, 0 replies; 6+ messages in thread
From: YunQiang Su @ 2024-01-12 14:33 UTC (permalink / raw)
  To: Vladimir Makarov; +Cc: gcc-patches

Vladimir Makarov <vmakarov@redhat.com> 于2024年1月11日周四 22:35写道:
>
> The following patch fixes
>
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112918
>
> The patch was successfully bootstrapped and tested on x86_64, aarch64,
> ppc64le

This patch causes some ICE on MIPS:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113354

PS: how to test cross build for mips:

1. apt install g++-multilib-mipsel-linux-gnu
2. apply patch:
https://gcc.gnu.org/pipermail/gcc-patches/2024-January/641619.html
3. ../configure --target=mipsel-linux-gnu \
      --includedir=/usr/mipsel-linux-gnu/include --enable-multilib \
      --with-arch-32=mips32r2 --with-fp-32=xx \
      --enable-multiarch --enable-targets=all \
      --with-arch-64=mips64r2 --prefix=/usr --disable-libsanitizer
4. make -j

-- 
YunQiang Su

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [pushed][PR112918][LRA]: Fixing IRA ICE on m68k
@ 2024-01-11 14:35 Vladimir Makarov
  2024-01-12 14:33 ` YunQiang Su
  0 siblings, 1 reply; 6+ messages in thread
From: Vladimir Makarov @ 2024-01-11 14:35 UTC (permalink / raw)
  To: gcc-patches

[-- Attachment #1: Type: text/plain, Size: 160 bytes --]

The following patch fixes

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112918

The patch was successfully bootstrapped and tested on x86_64, aarch64, 
ppc64le

[-- Attachment #2: pr112918.patch --]
[-- Type: text/x-patch, Size: 6087 bytes --]

commit 902a5931a1fbb04c65b48ca8b0f3827f6ff3b43e
Author: Vladimir N. Makarov <vmakarov@redhat.com>
Date:   Thu Jan 11 08:46:26 2024 -0500

    [PR112918][LRA]: Fixing IRA ICE on m68k
    
    Some GCC tests on m68K port of LRA is failed on `maximum number of
    generated reload insns per insn achieved`.  The problem is in that for
    subreg reload LRA can not narrow reg class more from ALL_REGS to
    GENERAL_REGS and then to data regs or address regs.  The patch permits
    narrowing reg class from reload insns if this results in successful
    matching of reg operand.  This is the second version of the patch to
    fix the PR.  This version adds matching with and without narrowing reg
    class and preferring match without narrowing classes.
    
    gcc/ChangeLog:
    
            PR rtl-optimization/112918
            * lra-constraints.cc (SMALL_REGISTER_CLASS_P): Move before in_class_p.
            (in_class_p): Restrict condition for narrowing class in case of
            allow_all_reload_class_changes_p.
            (process_alt_operands): Try to match operand without and with
            narrowing reg class.  Discourage narrowing the class.  Finish insn
            matching only if there is no class narrowing.
            (curr_insn_transform): Pass true to in_class_p for reg operand win.

diff --git a/gcc/lra-constraints.cc b/gcc/lra-constraints.cc
index da7e1748d75..6132cd9844a 100644
--- a/gcc/lra-constraints.cc
+++ b/gcc/lra-constraints.cc
@@ -261,6 +261,13 @@ enough_allocatable_hard_regs_p (enum reg_class reg_class,
   return false;
 }
 
+/* True if C is a non-empty register class that has too few registers
+   to be safely used as a reload target class.	*/
+#define SMALL_REGISTER_CLASS_P(C)		\
+  (ira_class_hard_regs_num [(C)] == 1		\
+   || (ira_class_hard_regs_num [(C)] >= 1	\
+       && targetm.class_likely_spilled_p (C)))
+
 /* Return true if REG satisfies (or will satisfy) reg class constraint
    CL.  Use elimination first if REG is a hard register.  If REG is a
    reload pseudo created by this constraints pass, assume that it will
@@ -318,7 +325,11 @@ in_class_p (rtx reg, enum reg_class cl, enum reg_class *new_class,
       common_class = ira_reg_class_subset[rclass][cl];
       if (new_class != NULL)
 	*new_class = common_class;
-      return enough_allocatable_hard_regs_p (common_class, reg_mode);
+      return (enough_allocatable_hard_regs_p (common_class, reg_mode)
+	      /* Do not permit reload insn operand matching (new_class == NULL
+		 case) if the new class is too small.  */
+	      && (new_class != NULL || common_class == rclass
+		  || !SMALL_REGISTER_CLASS_P (common_class)));
     }
 }
 
@@ -923,13 +934,6 @@ operands_match_p (rtx x, rtx y, int y_hard_regno)
    && GET_MODE_SIZE (MODE).is_constant ()	\
    && !targetm.cannot_force_const_mem (MODE, X))
 
-/* True if C is a non-empty register class that has too few registers
-   to be safely used as a reload target class.	*/
-#define SMALL_REGISTER_CLASS_P(C)		\
-  (ira_class_hard_regs_num [(C)] == 1		\
-   || (ira_class_hard_regs_num [(C)] >= 1	\
-       && targetm.class_likely_spilled_p (C)))
-
 /* If REG is a reload pseudo, try to make its class satisfying CL.  */
 static void
 narrow_reload_pseudo_class (rtx reg, enum reg_class cl)
@@ -2137,6 +2141,7 @@ process_alt_operands (int only_alternative)
   /* True if output stack pointer reload should be generated for the current
      alternative.  */
   bool curr_alt_out_sp_reload_p;
+  bool curr_alt_class_change_p;
   rtx op;
   /* The register when the operand is a subreg of register, otherwise the
      operand itself.  */
@@ -2223,6 +2228,7 @@ process_alt_operands (int only_alternative)
       early_clobbered_regs_num = 0;
       curr_alt_out_sp_reload_p = false;
       curr_reuse_alt_p = true;
+      curr_alt_class_change_p = false;
       
       for (nop = 0; nop < n_operands; nop++)
 	{
@@ -2247,6 +2253,7 @@ process_alt_operands (int only_alternative)
 	  bool scratch_p;
 	  machine_mode mode;
 	  enum constraint_num cn;
+	  bool class_change_p = false;
 
 	  opalt_num = nalt * n_operands + nop;
 	  if (curr_static_id->operand_alternative[opalt_num].anything_ok)
@@ -2630,9 +2637,16 @@ process_alt_operands (int only_alternative)
 				   (this_alternative_exclude_start_hard_regs,
 				    hard_regno[nop]))))
 			win = true;
-		      else if (hard_regno[nop] < 0
-			       && in_class_p (op, this_alternative, NULL))
-			win = true;
+		      else if (hard_regno[nop] < 0)
+			{
+			  if (in_class_p (op, this_alternative, NULL))
+			    win = true;
+			  else if (in_class_p (op, this_alternative, NULL, true))
+			    {
+			      class_change_p = true;
+			      win = true;
+			    }
+			}
 		    }
 		  break;
 		}
@@ -2647,6 +2661,15 @@ process_alt_operands (int only_alternative)
 	  if (win)
 	    {
 	      this_alternative_win = true;
+	      if (class_change_p)
+		{
+		  curr_alt_class_change_p = true;
+		  if (lra_dump_file != NULL)
+		    fprintf (lra_dump_file,
+			     "            %d Narrowing class: reject+=3\n",
+			     nop);
+		  reject += 3;
+		}
 	      if (operand_reg[nop] != NULL_RTX)
 		{
 		  if (hard_regno[nop] >= 0)
@@ -2675,7 +2698,7 @@ process_alt_operands (int only_alternative)
 			  reject++;
 			}
 		      if (in_class_p (operand_reg[nop],
-				      this_costly_alternative, NULL))
+				      this_costly_alternative, NULL, true))
 			{
 			  if (lra_dump_file != NULL)
 			    fprintf
@@ -3351,7 +3374,7 @@ process_alt_operands (int only_alternative)
 	  best_reload_sum = reload_sum;
 	  goal_alt_number = nalt;
 	}
-      if (losers == 0)
+      if (losers == 0 && !curr_alt_class_change_p)
 	/* Everything is satisfied.  Do not process alternatives
 	   anymore.  */
 	break;
@@ -4388,7 +4411,7 @@ curr_insn_transform (bool check_only_p)
 
 	if (REG_P (reg) && (regno = REGNO (reg)) >= FIRST_PSEUDO_REGISTER)
 	  {
-	    bool ok_p = in_class_p (reg, goal_alt[i], &new_class);
+	    bool ok_p = in_class_p (reg, goal_alt[i], &new_class, true);
 
 	    if (new_class != NO_REGS && get_reg_class (regno) != new_class)
 	      {

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2024-01-12 14:33 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-12-18 22:16 [pushed][PR112918][LRA]: Fixing IRA ICE on m68k Vladimir Makarov
2023-12-19  0:05 ` Jeff Law
2023-12-19  3:34   ` Jeff Law
2023-12-20 20:35   ` Joseph Myers
2024-01-11 14:35 Vladimir Makarov
2024-01-12 14:33 ` YunQiang Su

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