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

Sanjoy Das sanjoy at playingwithpointers.com
Tue Jul 29 01:08:44 PDT 2014


Totally bike-shedding comment on terminology: `StratifiedSets.h` uses terms that both leak the fact that we're manipulating memory expressions and aliasing information (`hasAliasingExternal`) and terms that try to hide it (`up`, `down`).  It may be clearer to rename either `hasAliasingExternal` to something more general (some property that is transitive in the "low" direction) or `up` / `down` to something more specific (`addressof` / `dereference`).

================
Comment at: lib/Analysis/StratifiedSets.h:135
@@ +134,3 @@
+//  %c = call i1 @SomeFunc()
+//  %k = select %c, %ap, %bp
+//  store %ad, %a
----------------
Should this be `%k = select %c, %ad, %bp`?

================
Comment at: lib/Analysis/StratifiedSets.h:422
@@ +421,3 @@
+    while (current->isRemapped()) {
+      current = &links[current->getRemapIndex()];
+      start->updateRemap(current->number);
----------------
Why only update `start` here?  Can't the whole sequence of `SetLink`s remapped to the final index?  If it is beneficial to `updateRemap` only `start`, why not do it outside the loop, once we have the first `SetLink` that isn't remapped?

================
Comment at: lib/Analysis/StratifiedSets.h:494
@@ +493,3 @@
+
+      // TODO: Maybe find a better way to do this than a forest of if
+      // statements?
----------------
If I understood the logic correctly here, maybe this can be written not as worklist algorithm, but simply as two loops, each in one direction?  So something structurally like this:

```
    StratifiedIndex linksIntoIdx = idx1, linksFromIdx = idx2;
    while (true) {
      auto &linksInto = linksAt(linksIntoIdx);
      auto &linksFrom = linksAt(linksFromIdx);
      linksInto.remapTo(linksFrom.number);

      if (linksInto.hasAbove() && linksFrom.hasAbove()) {
        linksIntoIdx = linksInto.getAbove();
        linksFromIdx = linksFrom.getAbove();
      } else {
        break;
      }
    }

    if (linksAt(linksIntoIdx).hasAbove() || linksAt(linksFromIdx).hasAbove()) {
      auto &topLink =
          linksAt(linksIntoIdx).hasAbove() ?
          linksAt(linksIntoIdx).getAbove() : linksAt(linksFromIdx).getAbove();
      topLink.setBelow(linksIntoIdx);
      linksAt(linksIntoIdx).setAbove(topLink);
    }

    // ... and a second loop in the below direction
```

If the logic cannot be expressed that way then a comment on why that is the case would help. :)

http://reviews.llvm.org/D4550






More information about the llvm-commits mailing list