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: [RFA] expand from SSA form (1/2)


Hi,

On Fri, 24 Apr 2009, Richard Guenther wrote:

> > ! Â map = init_var_map (num_ssa_names + 1);
> > --- 291,297 ----
> > ! Â map = init_var_map (num_ssa_names);
> 
> This piece is ok as obvious.  Please commit it separately.

This and the other two instances of num_ssa_names confusions committed as 
r146815.

The rest of the patch (with adjustments except as noted below)
committed as r146817.  I had to apply the below additional change to be 
able to build Ada, as we can't redirect (and hence split) EH edges in RTL 
land.  So I have to pre-split them when queing insns.

Reregstrapped on x86_64-linux.

I haven't dealt with with these comments:

> > + Â if (SA.values
> > + Â Â Â && TREE_CODE (lhs) == SSA_NAME
> > + Â Â Â && SA.values[SSA_NAME_VERSION (lhs)])
> > + Â Â lhs = gimple_assign_rhs_to_tree (SA.values[SSA_NAME_VERSION (lhs)]);
> 
> Do we really need the SA.values array here?  It seems that
> a bitmap would be enough, as the definition should be still
> reachable via SSA_NAME_DEF_STMT (lhs).  (Thanks Paolo
> for noticing this)

This should indeed be possible, but I want to do this as cleanup next 
week.

> > + #define SSAVAR(x) (TREE_CODE (x) == SSA_NAME ? SSA_NAME_VAR (x) : x)
> 
> This should be in some public place, ideally with a more sound name.  
> Any ideas?

No bright idea.  STRIP_SSA perhaps?  If someone has a better name or 
agrees to that one, I'd move it to tree.h and make a pass over the 
compiler to substitute the above pattern with the macro.

> > Âexecute_free_cfg_annotations (void)
> > Â{
> > - Â /* And get rid of annotations we no longer need. Â*/
> > - Â delete_tree_cfg_annotations ();
> > -
> 
> It looks like the pass referencing this is now dead.  Please remove
> this function and the pass structure.

It is, but done as followup.

> *************** struct var_ann_d GTY(())
> *** 234,243 ****
>    /* Used by var_map for the base index of ssa base variables.  */
>    unsigned base_index;
> 
> hmm, so what is needed to remove base_index as well?

Rewriting var_map_base_init() to not use var annotations.  SSA name 
coalescing and conflict building wants to have a DECL->dense-integer 
mapping for various reasons (two-stage approach for building 
conflicts).

Currently that mapping is generated by storing the next available index 
into the var annotation (to be able to read it out again when the same 
basevar is seen for a different partition).  But this whole info is 
strictly local to the above function, so it doesn't need to live in the 
annotation.  I could very well implement this as an array indexed by 
DECL_UID.  The UIDs shouldn't become exceptionally large, so that seems 
feasible.  I wouldn't want to use a hash-table for fear of slowing down 
var_map_base_init().

> +   NEXT_PASS (pass_mudflap_2);
>    NEXT_PASS (pass_mark_used_blocks);
> 
> pass_mark_unused_blocks is no longer needed now as you remove unused
> locals and that
> marks blocks used?  Please remove this pass (and its code).

Also followup.


Ciao,
Michael.

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