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]

Re: [Patch, ira] Invalid assert in reload1.c::finish_spills?


On 8/7/19 7:36 AM, SenthilKumar.Selvaraj@microchip.com wrote:
Hi,

   gcc/testsuite/c-c++-common/pr60101.c fails with an ICE for the
   avr target, because of a gcc_assert firing at reload1.c:4233

   The assert (in the patch below) looks bogus to me, as it's in
   the if block of

     if (! ira_conflicts_p || reg_renumber[i] >= 0)

   For this testcase and for the avr target, ira_conflicts_p is
   false because build_conflict_bit_table bailed out early
   (conflict table too big).
   If reg_renumber[i] is now negative, the assert fires and causes
   the ICE.

   Getting rid of the assert (patch below) makes the ICE go away,
   not sure if that's the right fix though.

   Comments?

Mike Matz is right.  Removing the assertion will make the bug even worse (changing memory beyond pseudo_previous_regs).

I did some investigation.  This bug occurred from a 10 years old patch avoiding building big conflict tables in IRA.  And the assert was in reload before IRA.

I think the solution should be

Index: reload1.c
===================================================================
--- reload1.c   (revision 273762)
+++ reload1.c   (working copy)
@@ -4225,13 +4225,8 @@ finish_spills (int global)
       spill_reg_order[i] = -1;

   EXECUTE_IF_SET_IN_REG_SET (&spilled_pseudos, FIRST_PSEUDO_REGISTER, i, rsi)
-    if (! ira_conflicts_p || reg_renumber[i] >= 0)
+    if (reg_renumber[i] >= 0)
       {
-       /* Record the current hard register the pseudo is allocated to
-          in pseudo_previous_regs so we avoid reallocating it to the
-          same hard reg in a later pass.  */
-       gcc_assert (reg_renumber[i] >= 0);
-
        SET_HARD_REG_BIT (pseudo_previous_regs[i], reg_renumber[i]);
        /* Mark it as no longer having a hard register home.  */
        reg_renumber[i] = -1;

So if the patch works, you can commit it to the trunk.

iff --git a/gcc/reload1.c b/gcc/reload1.c
index 38ee356a791..5acba706bee 100644
--- a/gcc/reload1.c
+++ b/gcc/reload1.c
@@ -4230,7 +4230,6 @@ finish_spills (int global)
  	/* Record the current hard register the pseudo is allocated to
  	   in pseudo_previous_regs so we avoid reallocating it to the
  	   same hard reg in a later pass.  */
-	gcc_assert (reg_renumber[i] >= 0);

  	SET_HARD_REG_BIT (pseudo_previous_regs[i], reg_renumber[i]);
  	/* Mark it as no longer having a hard register home.  */



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