[PATCH] LLVM CFL Alias Analysis -- Supporting Data Structures

hfinkel at anl.gov hfinkel at anl.gov
Sat Aug 23 09:20:50 PDT 2014


This is looks pretty good. Once the comments from the others reviewers are addressed, I think this can go in.

================
Comment at: lib/Analysis/StratifiedSets.h:195
@@ +194,3 @@
+// than doing this at each merge, we note in the BuilderLink structure that a
+// remap
+// has occurred, and use this information so we can defer renumbering set
----------------
Don't break the line here.

================
Comment at: lib/Analysis/WeightedBidirectedGraph.h:1
@@ +1,2 @@
+//===- WeightedBidirectedGraph.h - Abstract weighted, bidirected graph. ---===//
+//
----------------
Nick Lewycky wrote:
> Nick Lewycky wrote:
> > "Bidirected"? Bidirectional?
> I'm not convinced this should be pulled out of CFL Alias Analysis.
> 
> LLVM's graph infrastructure has:
>  a) generic algorithms that operate over child_begin/child_end iterators
>  b) wrappers that are constructed from other objects and provide child_begin/end.
> Sometimes the wrappers in (b) are null wrappers the implementation of the graph APIs are intrusive upon the underlying classes.
> 
> This doesn't fit that model at all, this is a graph data type. If this needs to exist at all (ie., we can't lazily construct it from other objects we have handy like the IR) then it should be local to CFLAliasAnalysis. That could mean creating a new CFGAlias directory to put this file in.
I agree. It is also not that big and could probably just go into the main source file too.

http://reviews.llvm.org/D4550






More information about the llvm-commits mailing list