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: Fix postreload liveness computation


> Unfortunately, not all live_at_start sets are valid before reload_combine:
>
>   /* Set up LABEL_LIVE and EVER_LIVE_AT_START.  The register lifetime
>      information is a bit fuzzy immediately after reload, but it's
>      still good enough to determine which registers are live at a jump
>      destination.  */
>
> I think what it means by "a bit fuzzy" is that the liveness is wrong
> in cases where reload might inherit across a block boundary.  In other
> words, the liveness is wrong for blocks that are only reached via
> fallthru edges.  So, rather than look at the live_at_end sets directly,
> reload_combine instead tries to work out the liveness from the known-valid
> live_at_start sets.  The problem is that it does so only for jumps;
> it doesn't consider exceptional edges.

I'm not a specialist of reload_combine, but I think that your interpretation 
is quite sound.

> This causes a miscompilation of eh_personality.cc on sh-elf with a patch I'm
> working on. 

Out of curiosity, which part and how?

> The CFG itself is valid at this point, so I think the fix is simply
> to compute the union of all successor non-fallthru live_at_start sets.

Sounds OK to me, but the implementation looks a litte convoluted. :-)

> +  /* Set up BB_LIVE_AT_START[B] to the set of hard registers live at
> +     the beginning of block B.  This information is only valid for
> +     the entry and exit blocks, or for those that start with labels.
> +     The live-at-start sets are no longer valid for blocks that are
> +     only reached via fallthrough; reload may have inherited from the
> +     previous block.  */
> +  bb_live_at_start = XNEWVEC (HARD_REG_SET, last_basic_block);
> +  FOR_ALL_BB (bb)
> +    {
> +      live = &bb_live_at_start[bb->index];
> +      REG_SET_TO_HARD_REG_SET (*live, bb->il.rtl->global_live_at_start);
> +      compute_use_by_pseudos (live, bb->il.rtl->global_live_at_start);
>      }

What about skipping the computation when it is not valid to match the comment?

  if (EDGE_COUNT (bb->preds) == 1
      && (EDGE_PRED ((bb), 0)->flags & EDGE_FALLTHRU) != 0)

> @@ -778,6 +766,35 @@ reload_combine (void)
>        if (! INSN_P (insn))
>  	continue;
>
> +      /* See if INSN is the end of a basic block.  */
> +      new_bb = BLOCK_FOR_INSN (insn);
> +      if (new_bb && new_bb != bb)
> +	{
> +	  bb = new_bb;

Why not use BB_END?

> +	  all_live = NULL;
> +	  FOR_EACH_EDGE (e, ei, new_bb->succs)
> +	    if ((e->flags & EDGE_FALLTHRU) == 0)
> +	      {
> +		live = &bb_live_at_start[e->dest->index];
> +		if (!all_live)
> +		  all_live = live;
> +		else
> +		  {
> +		    if (all_live != &all_live_tmp)
> +		      {
> +			COPY_HARD_REG_SET (all_live_tmp, *all_live);
> +			all_live = &all_live_tmp;
> +		      }
> +		    IOR_HARD_REG_SET (all_live_tmp, *live);
> +		  }
> +	      }
> +
> +	  if (all_live)
> +	    for (i = FIRST_PSEUDO_REGISTER - 1; i >= 0; --i)
> +	      if (TEST_HARD_REG_BIT (*all_live, i))
> +		reg_state[i].use_index = -1;
> +	}

Hum... so now that we have a nice HARD_REG_SET API, we don't use it?

Why not the straightforward following?

  CLEAR_HARD_REG_SET (...)

  FOR_EACH_EDGE (e, ei, new_bb->succs)
    if ((e->flags & EDGE_FALLTHRU) == 0)
       IOR_HARD_REG_SET (...)

  if (!hard_reg_set_empty_p (...))
    for (i = FIRST_PSEUDO_REGISTER - 1; i >= 0; --i)
      if (TEST_HARD_REG_BIT (..., i))
        ...

-- 
Eric Botcazou


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