[cfe-dev] Refactoring the dominators patch for Clang

[Guoping Long]

Hi, Anna

  First, thank your for your time for commenting this code!
  There is one more subtle difference. This is the major one that ended up
the code this way. It is the difference between CFGBlock::iterator and
Function::iterator, as shown in the code excerpt below.

Function::iterator. Each iterator I is used directly as BasicBlock * here.
      // Initialize the roots list
      for (typename FT::iterator I = F.begin(), E = F.end(); I != E; ++I) {
        if (std::distance(GraphTraits<FT*>::child_begin(I),
                          GraphTraits<FT*>::child_end(I)) == 0)
          addRoot(I);

CFG::iterator. Each iterator has the CFGBlock ** type. In this case we need
a pointer indirection here.
      // Initialize the roots list
      for (typename FT::iterator I = F.begin(), E = F.end(); I != E; ++I) {
        if (std::distance(GraphTraits<FT*>::child_begin(*I),
                          GraphTraits<FT*>::child_end(*I)) == 0)
          addRoot(*I);

One approach is to change CFG or Function implementation to make them have
the same interface. But I am afraid this may cause two many changes to the
code base. I would love to hear your comments. Is there a decent solution
that does not require too many changes to the code?

--
Guoping


2011/11/2 Anna Zaks <ganna at apple.com>

> Hi Guoping,
>
> It seems that the only reason why you need to copy the recalculate
> function is that the names for the entry block in the clang CFG and LLVM
> function are different. Is that correct? (The bool parameter does not seem
> to be used..)
>
> A simple solution to that would be to make sure that we have the same name
> in both. I suggest using getEntryBlock().
>
> On llvm's side, it would involve changing the recalculate function to use
> getEntryBlock instead of front(). Looks like they are the same thing.
>
> On Clang's side, we could just rename CFG::getEntry()  ->
> CFG::getEntryBlock().
>
> Thanks!
> Anna.
> On Oct 29, 2011, at 3:58 PM, Guoping Long wrote:
>
> > Hi, Ted
> >
> >    The preliminary refactoring of the dominators patch for clang based
> on the more efficient LLVM core implementation is done. Attached is the
> patch. I am not very satisfied with this version because it relies a ugly
> hack to deal with the subtle differences between LLVM Function and Clang
> CFG. Since this version requires some modifications to
> include/llvm/Analysis/Dominators.h, so there is also a patch for llvm.
> >
> >     While I believe there should be a cleaner way to do this, I do not
> know how to achieve that.  Please let me know your comments. I shall
> continue to improve until it become satisfactory.
> >     Regards.
> >
> > ----
> > Guoping
> >
> <dominators-clang.patch><dominators-llvm.patch>_______________________________________________
> > cfe-dev mailing list
> > cfe-dev at cs.uiuc.edu
> > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20111104/50a09861/attachment.html>