[PATCH] Scoped NoAlias Metadata

Andrew Trick atrick at apple.com
Fri Jan 10 23:12:32 PST 2014


  Hal, this looks pretty good!

  On the design, we could do better that a quadratic search on each query over two lists of AA scopes. But it doesn't seem worth the complexity at this time.


================
Comment at: lib/Analysis/ScopedNoAliasAA.cpp:82-85
@@ +81,6 @@
+
+namespace {
+  /// ScopedNoAliasAA - This is a simple alias analysis
+  /// implementation that uses scoped-noalias metadata to answer queries.
+  class ScopedNoAliasAA : public ImmutablePass, public AliasAnalysis {
+  public:
----------------
I personally don't like indenting large class definitions within anonymous namespace. It's unnecessary indentation and I find myself wondering if I'm looking at a nested class.

================
Comment at: lib/Analysis/ScopedNoAliasAA.cpp:136-141
@@ +135,8 @@
+
+/// Aliases - Test whether the scope represented by A may alias the
+/// scope represented by B. Specifically, A is the target scope, and B is the
+/// noalias scope.
+bool
+ScopedNoAliasAA::Aliases(const MDNode *A,
+                         const MDNode *B) const {
+  // Climb the tree from A to see if we reach B.
----------------
I know this is consistent with TBAA, but I find it misleading: Aliases() returns true on an unknown relation. Shouldn't it be called MayAlias()?

================
Comment at: lib/Analysis/ScopedNoAliasAA.cpp:168-174
@@ +167,9 @@
+
+  if (AScopes && BNoAlias)
+    for (unsigned i = 0, ie = AScopes->getNumOperands(); i != ie; ++i)
+      if (const MDNode *AMD = dyn_cast<MDNode>(AScopes->getOperand(i)))
+        for (unsigned j = 0, je = BNoAlias->getNumOperands(); j != je; ++j)
+          if (const MDNode *BMD = dyn_cast<MDNode>(BNoAlias->getOperand(j)))
+            if (!Aliases(AMD, BMD))
+              return NoAlias;
+
----------------
Please put this in a helper instead of repeating it four times:

e.g.

+static bool mayAliasInScopes(const MDNode *Scopes, const MDNode *NoAlias) {
+  if (!Scopes || !NoAlias)
+    return false;
+  for (unsigned i = 0, ie = Scopes->getNumOperands(); i != ie; ++i) {
+    if (const MDNode *SMD = dyn_cast<MDNode>(Scopes->getOperand(i)))
+      for (unsigned j = 0, je = NoAlias->getNumOperands(); j != je; ++j)
+        if (const MDNode *NAMD = dyn_cast<MDNode>(NoAlias->getOperand(j)))
+          if (!MayAlias(SMD, NAMD))
+            return false;
+  }
+  return true;


================
Comment at: test/Transforms/Inline/noalias2.ll:43
@@ +42,3 @@
+
+; Check that when hello() in inlined into foo(), and then foo() is inlined into
+; foo2(), the noalias scopes are properly concatenated.
----------------
is inlined


http://llvm-reviews.chandlerc.com/D2194



More information about the llvm-commits mailing list