Re: [RFA/RFC, IRA] Fix PR bootstrap/37859

[Adam Nemet <anemet at caviumnetworks dot com>]

Since then I finished regression-testing on {mips64octeon,x86_64}-linux-gnu.

On MIPS, because of the boostrap failure I of course don't have reference
testresults.  So I used
http://gcc.gnu.org/ml/gcc-testresults/2008-10/msg00581.html.

There were three new failures all unrelated to the patch.  The first was PR
middle-end/37669.  The second was PR fortran/37319.  The third was IRA-related
but even existed before the patch:

  PASS->FAIL (total: 4) 
    gcc.target/mips/dspr2-MULT.c scan-assembler ac2 
    gcc.target/mips/dspr2-MULT.c scan-assembler ac3 
    gcc.target/mips/dspr2-MULTU.c scan-assembler ac2 
    gcc.target/mips/dspr2-MULTU.c scan-assembler ac3 

OK to install?

(I also noticed that the ascii cfg below had tabs.  Below is an untabified
version.)

Adam

Adam Nemet <anemet@caviumnetworks.com> writes:
> This is a bootstrap failure on mips64octeon-linux-gnu.  try_forward_edges gets
> miscompiled by IRA which causes an ICE in stage3.
>
> This is the simplified CFG (I also have the dot file for the complete CFG with
> the relevant blocks annotated if someone is interested):
>
>         +-----> 110
>         |        |
>         |  a61   .
>         |  r696  .
>         |  h-1   .
>         |        |
>         |       15 <----------+
>         |        |            |
>         |        .  a256      |
>         |        .  r719   r719 = xmalloc (..)
>         |        .  h30       |
>         |        |            |
>         |       82 -----------+
>         |        |   +--------+
>         |        |   | a231   |
>         |        | <-+ r711   |
>         |       87 --+ h30    |
>         |        |   |        |
>         |        |   +--------+
>         +------ 88
>
> Arrows point downward unless marked otherwise.  For each loop I included an
> allocno and the corresponding pseudo register and hard register.  These
> allocnos all correspond to the same original pseudo (r284).
>
> The bug occurs because we omit the move from r711 to r696 on edge 87->88 and
> hence forget that we ever allocated the threaded_edges array.
>
> The reason the move is omitted becase not_modified_p in ira-emit.c returns
> true for a231 and a61.
>
>   /* Return TRUE if move of SRC_ALLOCNO to DEST_ALLOCNO does not change
>      value of the destination.  One possible reason for this is the
>      situation when SRC_ALLOCNO is not modified in the corresponding
>      loop.  */
>   static bool
>   not_modified_p (ira_allocno_t src_allocno, ira_allocno_t dest_allocno)
>   {
>     int regno, orig_regno;
>     ira_allocno_t a;
>     ira_loop_tree_node_t node;
>   
>     ira_assert (ALLOCNO_CAP_MEMBER (src_allocno) == NULL
>   	      && ALLOCNO_CAP_MEMBER (dest_allocno) == NULL);
>     orig_regno = ALLOCNO_REGNO (src_allocno);
>     regno = REGNO (ALLOCNO_REG (dest_allocno));
>     for (node = ALLOCNO_LOOP_TREE_NODE (src_allocno);
>          node != NULL;
>          node = node->parent)
>       if ((a = node->regno_allocno_map[orig_regno]) == NULL)
>         break;
>       else if (REGNO (ALLOCNO_REG (a)) == (unsigned) regno)
>         return true;
>       else if (bitmap_bit_p (node->modified_regnos, orig_regno))
>         return false;
>     return node != NULL;
>   }
>
> In the first iteration since r711 is unchanged in the loop starting at bb87 we
> go on.  In the second iteration we're looking at the same allocnos so we
> return true.  This is wrong however because the value of the allocno could
> have changed in this loop.
>
> My solution is to check whether the value is unmodified before checking
> equivalence of the allocnos.
>
> Testing is still ongoing but boostrapping has already finished on
> x86_64-linux-gnu and has passed stage2-3 comparison on mips64octeon-linux-gnu.
>
> I will report back with the result of the regtesting but in the meantime I'd
> like to get some feedback if my thinking is correct.
>
> Adam
>
>
> 	PR bootstrap/37859
> 	* ira-emit.c (not_modified_p): Only return true if register is not
> 	modified in the loop.
>
> Index: ira-emit.c
> ===================================================================
> --- ira-emit.c	(revision 141180)
> +++ ira-emit.c	(working copy)
> @@ -280,10 +280,10 @@ not_modified_p (ira_allocno_t src_allocn
>         node = node->parent)
>      if ((a = node->regno_allocno_map[orig_regno]) == NULL)
>        break;
> -    else if (REGNO (ALLOCNO_REG (a)) == (unsigned) regno)
> -      return true;
>      else if (bitmap_bit_p (node->modified_regnos, orig_regno))
>        return false;
> +    else if (REGNO (ALLOCNO_REG (a)) == (unsigned) regno)
> +      return true;
>    return node != NULL;
>  }
>