Fix typo in comment for function "static void initializeWorkList" in "/lib/Analysis/CFLAndersAliasAnalysis.cpp"

[Enna1]

Summary

It seems that there is a typo in "/lib/Analysis/CFLAndersAliasAnalysis.cpp".
There is a comment in function "static void initializeWorkList":

"If there's an assignment edge from X to Y, it means Y is reachable from X at S2 and X is reachable from Y at S1"

The corrected comment should be:
"If there's an assignment edge from X to Y, it means Y is reachable from X at S3 and X is reachable from Y at S1"

[Enna1]

The definition of function "initializeWorkList" :

static void initializeWorkList(std::vector<WorkListItem> &WorkList,
                               ReachabilitySet &ReachSet,
                               const CFLGraph &Graph) {
  for (const auto &Mapping : Graph.value_mappings()) {
    auto Val = Mapping.first;
    auto &ValueInfo = Mapping.second;
    assert(ValueInfo.getNumLevels() > 0);

    // Insert all immediate assignment neighbors to the worklist
    for (unsigned I = 0, E = ValueInfo.getNumLevels(); I < E; ++I) {
      auto Src = InstantiatedValue{Val, I};
      // If there's an assignment edge from X to Y, it means Y is reachable from
      // X at S3 and X is reachable from Y at S1
      for (auto &Edge : ValueInfo.getNodeInfoAtLevel(I).Edges) {
        propagate(Edge.Other, Src, MatchState::FlowFromReadOnly, ReachSet,
                  WorkList);
        propagate(Src, Edge.Other, MatchState::FlowToWriteOnly, ReachSet,
                  WorkList);
      }
    }
  }
}

MatchState::FlowFromReadOnly represents S1 in the paper "Demand-driven alias analysis for C", MatchState::FlowToWriteOnly and MatchState::FlowToReadWrite represent S3 in the paper.

[JonasToth]

i have no clue if that is correct. i added some reviews from the CSA guys.

*EDIT*: Please upload the diff with full context.

[NoQ]

That's not Clang though, so i don't know much about it.

I think @george.burgess.iv is the right person because he appears a lot in git blame :)

[george.burgess.iv]

I think @george.burgess.iv is the right person because he appears a lot in git blame :)

Yeah, I just need to go back through the paper and remember all of this so I can approve this with confidence. :)

Thanks for the patch! I hope to get to it later this week.

[george.burgess.iv]

LGTM; thanks again!

Do you need me to commit this?

[Enna1]

Yes, it's my first time to request a code review.
Thank a lot for your help to commit this.

llvm/lib/Analysis/CFLAndersAliasAnalysis.cpp

for (const auto &Mapping : Graph.value_mappings()) {
     auto Val = Mapping.first;
     auto &ValueInfo = Mapping.second;
     assert(ValueInfo.getNumLevels() > 0);
 
     // Insert all immediate assignment neighbors to the worklist
     for (unsigned I = 0, E = ValueInfo.getNumLevels(); I < E; ++I) {
       auto Src = InstantiatedValue{Val, I};
       // If there's an assignment edge from X to Y, it means Y is reachable from
-      // X at S2 and X is reachable from Y at S1
+      // X at S3 and X is reachable from Y at S1
       for (auto &Edge : ValueInfo.getNodeInfoAtLevel(I).Edges) {
         propagate(Edge.Other, Src, MatchState::FlowFromReadOnly, ReachSet,
                   WorkList);
         propagate(Src, Edge.Other, MatchState::FlowToWriteOnly, ReachSet,
                   WorkList);
       }
     }
   }