diff mbox series

[pushed,PR112918,LRA] : Fixing IRA ICE on m68k

Message ID 269d021e-7aad-22de-1469-f332abbfede9@redhat.com
State New
Headers show
Series [pushed,PR112918,LRA] : Fixing IRA ICE on m68k | expand

Commit Message

Vladimir Makarov Dec. 18, 2023, 10:16 p.m. UTC
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.

Comments

Jeff Law Dec. 19, 2023, 12:05 a.m. UTC | #1
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
Jeff Law Dec. 19, 2023, 3:34 a.m. UTC | #2
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
Joseph Myers Dec. 20, 2023, 8:35 p.m. UTC | #3
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.)
diff mbox series

Patch

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