On Thu, 17 Jul 2025, Filip Kastl wrote:

> On Thu 2025-07-17 11:48:33, Richard Biener wrote:
> > On Thu, 17 Jul 2025, Filip Kastl wrote:
> > 
> > > On Thu 2025-07-17 10:00:01, Richard Biener wrote:
> > > > On Thu, 17 Jul 2025, Filip Kastl wrote:
> > > > 
> > > > > Hi,
> > > > > 
> > > > > This patch cuts out the solver from tree-ssa-structalias.cc.  Soon, 
> > > > > I'd like to
> > > > > also cut out the part that generates constraints and perhaps also 
> > > > > other parts.
> > > > > 
> > > > > My big goal is to implement Steensgaard-style points-to analysis and 
> > > > > use it
> > > > > interprocedurally.  Unlike the IPA-PTA we currently have, Steensgaard 
> > > > > should be
> > > > > fast enough to be enabled by default.  Once this matures a bit, my 
> > > > > hope is that
> > > > > this could enable GCC to optimize in many interesting cases where it 
> > > > > didn't
> > > > > have enough alias information before.  But that is a very long-term 
> > > > > vision.
> > > > > 
> > > > > Cutting tree-ssa-structalias.cc into smaller files is preparatory 
> > > > > work.
> > > > > Even ignoring the upcoming changes, cutting a file of this size into 
> > > > > smaller
> > > > > files seems like a good idea to me.  I believe it will improve 
> > > > > readability.
> > > > > It will also be clearer what I'm modifying in each of the upcoming 
> > > > > IPA-PTA
> > > > > patches.  For example, I'm not planning to modify the existing solver 
> > > > > at all.
> > > > > The splitting will also make it easier to eventually plug in the new
> > > > > Steensgaard solver.
> > > > > 
> > > > > I've tried to make as few changes as seemed reasonable.  I go into 
> > > > > more detail
> > > > > about how I did the splitting in the commit message.
> > > > > 
> > > > > I've named the new files *-andersen.{cc,h} because the current solver 
> > > > > would be
> > > > > refered to as Andersen-style in points-to analysis literature.  When 
> > > > > I'll be
> > > > > adding the Steensgard-style solver, I'm planning to call those files
> > > > > *-steens.{cc,h}.
> > > > > 
> > > > > Btw, I copied the license comment at the top of 
> > > > > tree-ssa-structalias.cc into
> > > > > all the files I created.  This means that it says "Copyright (C) 
> > > > > 2005-2025" in
> > > > > files which were just created.  Is that ok?
> > > > > 
> > > > > Bootstrapped and regtested on x86_64.  Ok to push?
> > > > 
> > > > I'm OK with splitting up the file, but can we get rid of
> > > > 'tree' and 'structalias'?  I'd say ssa-pta or for the solver,
> > > > which has nothing to do with SSA, use pta-andersen.{cc,h} instead?
> > > > At least for the new file, don't rename the old one.
> > > 
> > > Yes, I'm all for that.  I thought about doing that in a later patch but I 
> > > guess
> > > it will be nicer to do that now.
> > > 
> > > > That extends to the choice of the namespace name as well (I'd use 
> > > > 'pta').
> > > 
> > > Ok.
> > > 
> > > > The constraint building is mostly gimple based (it doesn't use SSA),
> > > > so gimple-pta-constraints.{cc,h} might be appropriate and the pass
> > > > itself would be ssa-pta.{cc,h} since it puts the solutions on SSA
> > > > vars only. 
> > > 
> > > You say we should keep tree-ssa-structalias.cc and that the pass (the 
> > > alias,
> > > ealias and ipa-pta passes, right?) should be in ssa-pta.{cc,h}.  So you 
> > > suggest
> > > splitting out the pass classes into ssa-pta.{cc,h}?  I'm confused about 
> > > which
> > > parts you think should then be in tree-ssa-structalias.cc and which in
> > > ssa-pta.cc.
> > 
> > For now those pieces would remain in tree-ssa-structalias.cc.  After all
> > reorg is complete we'd move that to ssa-pta.{cc,h}.  So I suggest to
> > defer this part until it gets clear how this all plays out.
> 
> Ah, ok.  I see.
> 
> > > Also, why not also rename tree-ssa-structalias.cc to be consistent?  Is 
> > > it a
> > > no-no to rename old files?
> > 
> > See above.
> > 
> > > If we ignored the fact that the constraint building doesn't really 
> > > interact
> > > with SSA (but AFAIU, it benefits from SSA) and that the passes don't 
> > > really
> > > modify gimple statements, we could have:
> > > 
> > > pta-andersen.{cc,h}
> > > pta-steens.{cc,h}
> > > gimple-ssa-pta-constraints.{cc,h}
> > > gimple-ssa-pta.{cc,h}
> > > 
> > > That seems nicer to me.  A justification for calling those files 
> > > gimple-ssa-*
> > > could be "to convey that this analysis happens on GIMPLE SSA IR".  But 
> > > you are
> > > the boss, of course.
> > 
> > Sure, that works for me and follows existing practice.
> > 
> > > Btw, I also thought about renaming the passes.  ipa-pta is fine.  But 
> > > ealias
> > > and alias could be called epta and pta.  That would convey what they are
> > > actually doing better, I think.  It would also be more consistent with 
> > > file
> > > names.
> > 
> > Yeah, but that's a separate change affecting dump file names and
> > -fdisable-tree-XYZ only.
> 
> I'll do that as a separate patch, then.
> 
> 
> I'll write a second version of this patch where I rename
> tree-ssa-structalias-andersen.{cc,h} to pta-andersen.{cc,h} and the 
> structalias
> namespace to pta (and update the commit message accordingly).  Can I consider
> that pre-approved?

Yes.

Richard.

> Thanks,
> Filip Kastl
> 

-- 
Richard Biener <rguent...@suse.de>
SUSE Software Solutions Germany GmbH,
Frankenstrasse 146, 90461 Nuernberg, Germany;
GF: Ivo Totev, Andrew McDonald, Werner Knoblich; (HRB 36809, AG Nuernberg)

Reply via email to