Re: [patch] 19/n: trans-mem: middle end/misc patches (LAST PATCH)

[Aldy Hernandez <aldyh at redhat dot com>]

> > Actually, the table organization is irrelevant, because upon
> > registering of the table in the runtime, we qsort the entire thing.
>
> False.  You get the equivalent of bootstrap comparison mismatches.
> If we actually used tm during the bootstrap.
>
> The simplest thing to do is to change the hash this table uses.
> E.g. use the DECL_UID right from the start, rather than the pointer.

Argh, will fix in a followup patch.

> > > > -          if (!computed_goto_p (stmt))
> > > > +         if (!computed_goto_p (stmt))
> > > >              {
> > > > -             tree new_dest = main_block_label (gimple_goto_dest (stmt));
> > > > -             gimple_goto_set_dest (stmt, new_dest);
> > > > +             label = gimple_goto_dest (stmt);
> > > > +             new_label = main_block_label (label);
> > > > +             if (new_label != label)
> > > > +               gimple_goto_set_dest (stmt, new_label);
> > >
> > > What's the reason for this changes?  Optimization?
> >
> > Yes.  Rth can elaborate if you deem necessary.
>
> Really?  I have no idea what this change achieves.
> I actually wonder if this is a merge error.

I won't complain :).  I have reverted the original patch and am 
including it in the final (attched) version I will commit.

> > > > +    case GIMPLE_TRANSACTION:
> > > > +      /* The ABORT edge has a stored label associated with it, otherwise
> > > > +        the edges are simply redirectable.  */
> > > > +      /* ??? We don't really need this label after the cfg is created.  */
> > > > +      if (e->flags == 0)
> > > > +       gimple_transaction_set_label (stmt, gimple_block_label (dest));
> > >
> > > So why set it (and thus keep it live)?
> >
> > This seems like leftovers from a previous incantation.  However, I'm not 100% sure, so I have disabled the code, but left it in a comment.  A full bootstrap/regtest revealed no regressions.
> >
> > rth, do you have any objections to remove this?
>
> I think that the comment is wrong.  We need that edge, and the label updated until pass_tm_edges, at which point the GIMPLE_TRANSACTION itself goes away.  Thus that label is live throughout the live of the GIMPLE_TRANSACTION node.
>
> Delete that ??? comment instead.

Done.

> Patch is otherwise ok.

Attched is the final revision of the patch.  I will commit once tests 
finish.

thank you.

Attachment: curr
Description: Text document