[PATCH] D22429: [CFLAA] Teach CFLAndersAliasAnalysis to discern reads from writes

[Jia Chen via llvm-commits]

grievejia marked 2 inline comments as done.

================
Comment at: lib/Analysis/CFLAndersAliasAnalysis.cpp:92
@@ +91,3 @@
+  FlowToWriteOnly,
+  FlowToReadWrite,
+  // The following two states together represent S4 in the paper
----------------
george.burgess.iv wrote:
> grievejia wrote:
> > george.burgess.iv wrote:
> > > Out of curiosity, do we gain anything from having `FlowToWriteOnly` on something that already has `FlowToReadWrite` on it?
> > > 
> > > Specifically, given:
> > > 
> > > ```
> > > ReachabilitySet R = whatever;
> > > R.insert(FromVal, ToVal, FlowToReadWrite);
> > > R.insert(FromVal, ToVal, FlowToWriteOnly);
> > > ```
> > > 
> > > Is the second insert at all meaningful to us? If not, can we add a FIXME or TODO somewhere noting that we may be able to do less propagations if we check for this later?
> > > 
> > > (Same question for `FlowFromMemAliasNoReadWrite` vs `FlowFromMemAliasReadOnly`, and `FlowToMemAliasWriteOnly` vs `FlowToMemAliasReadWrite`)
> > The state FlowToWriteOnly does carry important information: if x is reachable from y at FlowToWriteOnly state, we know that y is (transitively) assigned to x. If x is reachable from y at FlowToReadWrite state, then there's a third value z which gets assigned to both x and y. The distinction here is basically what motivates this patch.
> Ahh, now that makes sense. Thanks for clarifying!
Now that you mentioned it, maybe XXXReadWrite is not a good name for those states. At a first glance it looks like if X is reachable from Y at XXXReadWrite states it means that X is both read from and written to Y, yet this is not true: what it really means is that the path from X to Y contains both assignment edges and reverse assignment edges, which implies a third value written to both. 

Let me add some comments that try to clarify this.




https://reviews.llvm.org/D22429