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: [RFC] [alias, middle-end] Alias and data dependency export


On Thu, 28 May 2009, Andrey Belevantsev wrote:

> Hello,
> 
> I have made some of the fixes, so more questions/RFC follow.
> 
> - delaying delete_tree_ssa until pass_clean_state just works, the loop over
> gimple bbs marked with FIXME in there can be removed, and bootstrap still
> works (testing is in progress now);

Good.

> - relying on SSA_NAME_PTR_INFO instead of saving it separately works, too,
> with the only exception of TMR_ORIGINALs, which can be of INDIRECT_REFs to
> VAR_DECLs, which ICEs in oracle helpers when trying to get points-to info.
> This works if I bail out from helpers when seeing not SSA_NAME and asserting
> we are not in GIMPLE.  Besides these changes to the helpers, all other oracle
> changes are gone.  Testing is in progress, too.

I think we should instead benchmark getting rid of TMR_ORIGINAL
completely.  We should be able to initialize mem_attrs from the
other fields of the TMR.  But that can be done as a followup if
the special casing is not too ugly for now (was there supposed to
be a patch attached?).

> - stack slot partition handling.  Your pseudocode assumes that there is a
> decl-to-partition mapping, which is not the case.  The code within
> record_stack_partition_for actually tries to create such mapping and uses it
> when recording expressions.  It would do what you want if I make sure that we
> fix points-to sets once per SSA_NAME instead of once per expressions.  I agree
> that the part unifying points-to sets of pointers that got partitioned into
> the same stack slot is unneeded though, so I removed it.  Again, testing is in
> progress.
> 
> Other choice would be to create and maintain such bitmaps during partitioning
> process, or to build them exclusively for this task and get rid of them
> afterwards; I can do either of this if it seems cleaner to you, but currently
> I don't see a big difference.

Yeah, I would like to see it done during the partitioning itself ...
but see below.

> - finally, rewriting bases to INDIRECT_REFs (the only major change left) vs.
> MEM_EXPR/MEM_ORIG_EXPR problem.  Looking at how the old MEM_EXPR field is
> used, I see that the decl from it is used within var-tracking.c (in addition
> to nonoverlapping_memrefs_p in alias.c), so we'd probably spoil debug info
> while doing rewrite (either to base partition decl, like now, or to
> INDIRECT_REF).  How important is this?

Important enough I think.  Does it only look at the base decl?  If so
I would add a mem_attrs.decl field just for this purpose (like we
have in reg_attrs).  Other opinions welcome.

I find mem_attrs mildly confusing anyway

typedef struct GTY(()) mem_attrs
{
  tree expr;                    /* expr corresponding to MEM.  */
  rtx offset;                   /* Offset from start of DECL, as 
CONST_INT.  */

so, is offset relative to &expr or relative to &get_base_address(expr)?
Or even relative to the MEM base?

> Also, if I would fail making rewrite work on-the-fly with bases from
> expand_stack_vars and from expand for other exprs, I'd probably need an extra
> walk to see all MEMs right after expand.  (Should not be too bad as I killed
> one walk in delete_tree_ssa :)  Or we need to delay expanding any variables
> until partition info is computed.

Well, again I think it should all be done during partitioning which is
done before expand_stack_vars.  Late expanded decls should never end
up in a partition anyway (and usually also not on the stack) - but maybe
I am missing something here ;)

Thanks,
Richard.


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