[CFLAA] Cleaned up StratifiedAttrs handling

[grievejia]

Summary

This revision patched up three different things regarding StratifiedAttrs.

1. There are two overloaded version of StratifiedSets::noteAttribute(): one version accepts an unsigned index as its second argument, and the other one accepts a bitset. The two overloads perform slightly different operations. For some reason, when StratifiedSets::noteAttribute() is invoked with an unsigned integer (the intention was to invoke the first overloaded version), my toolchain (clang-3.8 + libstdc++-5.3) always construct a bitset out of the given integer and invoke the second overloaded version. The wrongly resolved overload function call leads to subtle error in StratifiedAttrs computation, and the error is not caught by any of the existing regression tests.

To eliminate the potential ambiguity in overload resolution, the patch gets rid of the first overloaded version and make every caller of noteAttribute() to call the second overloaded version instead.

2. Previously, AttrUnknown and AttrAll were treated in the same way. There was no practical differences between the two. This patch removes AttrAll competely and relies on AttrUnknown to label values that are not understood by CFLAA.

3. A new StratifiedAttr, called "AttrEscaped", is added. The motivation for this attribute is to make the analysis more precise by distinguish between "values that escape the current function" and "values that do not escape the current function". The major difference between the two classes of values is that the former class does not alias global/argument/unknown values the the latter class may alias them. Consider this example:
   
   define void @foo() { %a = alloca i32 %b = alloca i32 %i = ptrtoint %a ... }

Here we cast %a to an integer and thus let it escape from the world that CFLAA can understand. CFLAA was conservative on cases like this: as long as a value escapes, the value will be marked as unknown. Therefore the analysis concludes that %a and %b may alias each other (but it is obvious from the function body that they don't).

We can be more aggressive about it because an escaped value isn't completely opaque to CFLAA: the analysis still keeps track of the value. A value escapes only means that it is possible for other opaque values to alias it.

This patch uses AttrEscaped for ptrtoint only. Our ultimate goal is to improve upon interprocedural case like this:

; Assume @external_func is opaque in the current translation unit
declare @external_func(i32*, i32*)

define void @foo() {
  %a = alloca i32
  %b = alloca i32
  call void @external_func(i32* %a, i32* %b)
  ret void
}

Here %a and %b escaped @foo yet they should not be aliases. CFLAA currently cannot prove this. The idea is to mark %a and %b as AttrEscaped, but we need additional mechanisms to guarantee soundness -- they will be added in subsequent patches.

[george.burgess.iv]

Thanks for the patch!

A few high-level things:

• This needs tests. Ideally at least one for each logical change that's being made (though I'm not sure how testable the overload-related issue would be). :)
• We generally request that people try to make reviews as targeted as possible. So, if you could please split patches like these out into N different reviews in the future, that would be great!

---

There are two overloaded version of StratifiedSets::noteAttribute

One is noteAttribute, the other is noteAttributes, so I don't think overload resolution should be kicking in at all here. :)

lib/Analysis/CFLAliasAnalysis.cpp

1100 ↗

(On Diff #59657)

This looks a bit subtle. I think we can make it less so if we move the index number check above this. If we do that, it seems that we can simplify this entire if/else chain to something like

if (a.none() || b.none() || (a == Escaped && b == Escaped))
  return NoAlias;
return MayAlias;

Right?

1102 ↗

(On Diff #59657)

Style nit:

if (foo)
  return bar;
if (baz)
  ...

Is preferred over

if (foo)
  return bar;
else if (baz)
  ...

1105 ↗

(On Diff #59657)

AttrsA.test(AttrEscapedIndex) && AttrsB.test(AttrEscapedIndex)?

Either way, this looks like it'll hand us NoAlias if I have e.g. two sets marked with every StratifiedAttr except AttrUnknown.

lib/Analysis/StratifiedSets.h

436 ↗

(On Diff #59657)

Does Link.setAttr have any uses aside from this one? If not, we should remove it, too. :)

[grievejia]

lib/Analysis/CFLAliasAnalysis.cpp

1100 ↗

(On Diff #59657)

If both a.none() and b.none() are true, we still need to check SetA.Index == SetB.Index, right?

1105 ↗

(On Diff #59657)

If we go with Attrs.test(AttrEscapedIndex), the problem is that we may miss the check for global/argument attrs. They are essentially the same as AttrUnknown except that we are aware why they are unknown.

Maybe I'll restructure the checks a little bit here and try to merge the check of global/argument/AttrUnknown together, to make it more clear.

[george.burgess.iv]

lib/Analysis/CFLAliasAnalysis.cpp

1100 ↗

(On Diff #59657)

Yup! Which is why I said we should move the index check above this. :)

[grievejia]

Updated aliasing logic.

Test case added.

[grievejia]

In D21000#450028, @george.burgess.iv wrote:

• This needs tests. Ideally at least one for each logical change that's being made

This is my mistake. I have no excuse for it :(

• We generally request that people try to make reviews as targeted as possible. So, if you could please split patches like these out into N different reviews in the future, that would be great!

OK I'll try to do that in the future.

There were two reasons why I did this giant patch: (1) I thought they are all related to StratifiedAttrs and share a common theme. (2) I haven't figure out how to work on patches that depends on each other, so I tend to use giant patch to minimize inter-patch dependency. Say I break this patch into two pieces P0 and P1, where P1 depends on P0. If for some reasons P0 get changed before it is committed in-tree, is there a clean way I can propagate the changes to P1 as well?

One is noteAttribute, the other is noteAttributes, so I don't think overload resolution should be kicking in at all here. :)

No wonder!

Then the problem could have been solved in a far less intrusive manner. Well, I'll just leave the changes as-is because I'm too lazy to manually revert all those changes...

[grievejia]

lib/Analysis/CFLAliasAnalysis.cpp

1104–1111 ↗

(On Diff #59791)

Aha, I see what you are saying. But I'm not sure moving the index check will simplify the logic here.

In general we have four types of sets: (1) AttrNone (2) AttrUnknown is set (3) only AttrEscaped is set (4) Any global/argument attr is set.

Let's perform a case-by-case analysis:

• (1) x (1): index check
• (1) x (2): NoAlias
• (1) x (3): NoAlias
• (1) x (4): NoAlias
• (2) x (2): MayAlias
• (2) x (3): MayAlias
• (2) x (4): MayAlias
• (3) x (3): index check
• (3) x (4): NoAlias
• (4) x (4): MayAlias

The puzzle here is to figure out the clearest way to express the above logic. I agree with you that I may not have the best answer, but simply putting the index check on top is not a panacea either. (Or I misunderstood what you're saying...)

[grievejia]

lib/Analysis/CFLAliasAnalysis.cpp

1104–1111 ↗

(On Diff #59791)

Oops, I made a mistake: (1) x (3) should be "index check" rather than "NoAlias". I'll update the codes accordingly.

[grievejia]

Updated aliasing logic

[george.burgess.iv]

Looks good after a few more comments.

This is my mistake. I have no excuse for it :(

If no one made mistakes, code reviews wouldn't exist. Just do your best. :)

Say I break this patch into two pieces P0 and P1, where P1 depends on P0. If for some reasons P0 get changed before it is committed in-tree, is there a clean way I can propagate the changes to P1 as well?

I use git branches. Whenever I change branch P0, I swap to P1 and rebase. It's not the prettiest solution, and sometimes requires effort, but it's the best thing I know of. If you use svn, I can't help you; my svn directory literally only exists so I can throw a patch on it and commit said patch.

Well, I'll just leave the changes as-is because I'm too lazy to manually revert all those changes

Yeah, the refactored code seems less error-prone anyway, so I'm okay with that.

lib/Analysis/CFLAliasAnalysis.cpp

116 ↗

(On Diff #59795)

Style nits: attr -> Attr, and we generally try to keep "static" functions static and out of unnamed namespaces.

117 ↗

(On Diff #59795)

Should be return attr.none() || attr == AttrEscaped;?

1095–1114 ↗

(On Diff #59795)

The reason I'm fond of moving the index check up is that it obviates the need for "index check" in your table. Those all get to turn into NoAliases, and it's one less thing to worry about when reading this attribute logic. :)

Either way, I didn't think the (3) x (4) case was NoAlias when I originally commented (...though I suspect we'll need to make it MayAlias in the near future, but we'll get there when we get there), so this is a bit more complex than I originally thought. What you have now seems correct to me, so I'm happy with it.

1113 ↗

(On Diff #59795)

Nit: No else. :)

[grievejia]

Stylish correction.

... And yet another aliasing logic update.

[grievejia]

lib/Analysis/CFLAliasAnalysis.cpp

1099–1117 ↗

(On Diff #59884)

Can you explain your suspicion that "we'll need to make it (3 x 4) MayAlias in the near future"? I thought the definition of AttrEscaped has already eliminate this possibility, since (3) only includes locally identifiable memory objects (allocas and mallocs) and those objects shouldn't alias any global/argument.

[george.burgess.iv]

LGTM -- will commit.

lib/Analysis/CFLAliasAnalysis.cpp

1099–1117 ↗

(On Diff #59884)

It's my understanding that our plan is to eventually mark call args as AttrEscaped instead of AttrUnknown. With everything in its current state, it seems that doing so would break in a case like:

int *G;
// assume this is external to CFLAA
void f(uintptr_t A) {
  G = (int *)A;
}

void callF() {
  int S;
  int *P = &S;
  f((uintptr)P);
  int *PAlias = G;
}

If we're not doing this, then (3 x 4) can stay NoAlias. :)

[grievejia]

lib/Analysis/CFLAliasAnalysis.cpp

1099–1117 ↗

(On Diff #59884)

I see the problem here. Thanks for the example. It's very instructive.

llvm/trunk/lib/Analysis/CFLAliasAnalysis.cpp

 
 const StratifiedIndex StratifiedLink::SetSentinel =
     std::numeric_limits<StratifiedIndex>::max();
 
 namespace {
 /// StratifiedInfo Attribute things.
 typedef unsigned StratifiedAttr;
 LLVM_CONSTEXPR unsigned MaxStratifiedAttrIndex = NumStratifiedAttrs;
-LLVM_CONSTEXPR unsigned AttrAllIndex = 0;
-LLVM_CONSTEXPR unsigned AttrGlobalIndex = 1;
-LLVM_CONSTEXPR unsigned AttrUnknownIndex = 2;
+LLVM_CONSTEXPR unsigned AttrEscapedIndex = 0;
+LLVM_CONSTEXPR unsigned AttrUnknownIndex = 1;
+LLVM_CONSTEXPR unsigned AttrGlobalIndex = 2;
 LLVM_CONSTEXPR unsigned AttrFirstArgIndex = 3;
 LLVM_CONSTEXPR unsigned AttrLastArgIndex = MaxStratifiedAttrIndex;
 LLVM_CONSTEXPR unsigned AttrMaxNumArgs = AttrLastArgIndex - AttrFirstArgIndex;
 
 LLVM_CONSTEXPR StratifiedAttr AttrNone = 0;
+LLVM_CONSTEXPR StratifiedAttr AttrEscaped = 1 << AttrEscapedIndex;
 LLVM_CONSTEXPR StratifiedAttr AttrUnknown = 1 << AttrUnknownIndex;
-LLVM_CONSTEXPR StratifiedAttr AttrAll = ~AttrNone;
+LLVM_CONSTEXPR StratifiedAttr AttrGlobal = 1 << AttrGlobalIndex;
 
 /// StratifiedSets call for knowledge of "direction", so this is how we
 /// represent that locally.
 enum class Level { Same, Above, Below };
 
 /// Edges can be one of four "weights" -- each weight must have an inverse
 /// weight (Assign has Assign; Reference has Dereference).
 enum class EdgeType {
GetEdgesVisitor(CFLAAResult &AA, SmallVectorImpl<Edge> &Output,
       : AA(AA), Output(Output), TLI(TLI) {}
 
   void visitInstruction(Instruction &) {
     llvm_unreachable("Unsupported instruction encountered");
   }
 
   void visitPtrToIntInst(PtrToIntInst &Inst) {
     auto *Ptr = Inst.getOperand(0);
-    Output.push_back(Edge(Ptr, Ptr, EdgeType::Assign, AttrUnknown));
+    Output.push_back(Edge(Ptr, &Inst, EdgeType::Assign, AttrEscaped));
   }
 
   void visitIntToPtrInst(IntToPtrInst &Inst) {
     auto *Ptr = &Inst;
     Output.push_back(Edge(Ptr, Ptr, EdgeType::Assign, AttrUnknown));
   }
 
   void visitCastInst(CastInst &Inst) {
public:
   void visitVAArgInst(VAArgInst &Inst) {
     // We can't fully model va_arg here. For *Ptr = Inst.getOperand(0), it does
     // two things:
     //  1. Loads a value from *((T*)*Ptr).
     //  2. Increments (stores to) *Ptr by some target-specific amount.
     // For now, we'll handle this like a landingpad instruction (by placing the
     // result in its own group, and having that group alias externals).
     auto *Val = &Inst;
-    Output.push_back(Edge(Val, Val, EdgeType::Assign, AttrAll));
+    Output.push_back(Edge(Val, Val, EdgeType::Assign, AttrUnknown));
   }
 
   static bool isFunctionExternal(Function *Fn) {
     return Fn->isDeclaration() || !Fn->hasLocalLinkage();
   }
 
   /// Gets whether the sets at Index1 above, below, or equal to the sets at
   /// Index2. Returns None if they are not in the same set chain.
for (auto *Fn : Fns) {
           auto MaybeRelation =
               getIndexRelation(Sets, ParamInfo.Index, RetInfo.Index);
           if (MaybeRelation.hasValue()) {
             AddEdge = true;
             Externals |= RetAttrs | ParamAttrs;
           }
         }
         if (AddEdge)
-          Output.push_back(Edge(FuncValue, ArgVal, EdgeType::Assign,
-                                StratifiedAttrs().flip()));
+          Output.push_back(
+              Edge(FuncValue, ArgVal, EdgeType::Assign, Externals));
       }
 
       if (Parameters.size() != Arguments.size())
         return false;
 
       /// Adding edges between arguments for arguments that may end up aliasing
       /// each other. This is necessary for functions such as
       /// void foo(int** a, int** b) { *a = *b; }
template <typename InstT> void visitCallLikeInst(InstT &Inst) {
     }
 
     // Because the function is opaque, we need to note that anything
     // could have happened to the arguments, and that the result could alias
     // just about anything, too.
     // The goal of the loop is in part to unify many Values into one set, so we
     // don't care if the function is void there.
     for (Value *V : Inst.arg_operands())
-      Output.push_back(Edge(&Inst, V, EdgeType::Assign, AttrAll));
+      Output.push_back(Edge(&Inst, V, EdgeType::Assign, AttrUnknown));
     if (Inst.getNumArgOperands() == 0 &&
         Inst.getType() != Type::getVoidTy(Inst.getContext()))
-      Output.push_back(Edge(&Inst, &Inst, EdgeType::Assign, AttrAll));
+      Output.push_back(Edge(&Inst, &Inst, EdgeType::Assign, AttrUnknown));
   }
 
   void visitCallInst(CallInst &Inst) { visitCallLikeInst(Inst); }
 
   void visitInvokeInst(InvokeInst &Inst) { visitCallLikeInst(Inst); }
 
   /// Because vectors/aggregates are immutable and unaddressable, there's
   /// nothing we can do to coax a value out of them, other than calling
void visitInsertElementInst(InsertElementInst &Inst) {
     Output.push_back(Edge(&Inst, Vec, EdgeType::Assign, AttrNone));
     Output.push_back(Edge(&Inst, Val, EdgeType::Dereference, AttrNone));
   }
 
   void visitLandingPadInst(LandingPadInst &Inst) {
     // Exceptions come from "nowhere", from our analysis' perspective.
     // So we place the instruction its own group, noting that said group may
     // alias externals
-    Output.push_back(Edge(&Inst, &Inst, EdgeType::Assign, AttrAll));
+    Output.push_back(Edge(&Inst, &Inst, EdgeType::Assign, AttrUnknown));
   }
 
   void visitInsertValueInst(InsertValueInst &Inst) {
     auto *Agg = Inst.getOperand(0);
     auto *Val = Inst.getOperand(1);
     Output.push_back(Edge(&Inst, Agg, EdgeType::Assign, AttrNone));
     Output.push_back(Edge(&Inst, Val, EdgeType::Dereference, AttrNone));
   }

 typedef WeightedBidirectionalGraph<std::pair<EdgeType, StratifiedAttrs>> GraphT;
 typedef DenseMap<Value *, GraphT::Node> NodeMapT;
 }
 
 //===----------------------------------------------------------------------===//
 // Function declarations that require types defined in the namespace above
 //===----------------------------------------------------------------------===//
 
-/// Given an argument number, returns the appropriate Attr index to set.
-static StratifiedAttr argNumberToAttrIndex(unsigned ArgNum);
+/// Given a StratifiedAttrs, returns true if it marks the corresponding values
+/// as globals or arguments
+static bool isGlobalOrArgAttr(StratifiedAttrs Attr);
 
-/// Given a Value, potentially return which AttrIndex it maps to.
-static Optional<StratifiedAttr> valueToAttrIndex(Value *Val);
+/// Given an argument number, returns the appropriate StratifiedAttr to set.
+static StratifiedAttr argNumberToAttr(unsigned ArgNum);
+
+/// Given a Value, potentially return which StratifiedAttr it maps to.
+static Optional<StratifiedAttr> valueToAttr(Value *Val);
 
 /// Gets the inverse of a given EdgeType.
 static EdgeType flipWeight(EdgeType Initial);
 
 /// Gets edges of the given Instruction*, writing them to the SmallVector*.
 static void argsToEdges(CFLAAResult &, Instruction *, SmallVectorImpl<Edge> &,
                         const TargetLibraryInfo &);
 

 
 static bool hasUsefulEdges(ConstantExpr *CE) {
   // ConstantExpr doesn't have terminators, invokes, or fences, so only needs
   // to check for compares.
   return CE->getOpcode() != Instruction::ICmp &&
          CE->getOpcode() != Instruction::FCmp;
 }
 
-static Optional<StratifiedAttr> valueToAttrIndex(Value *Val) {
+static bool isGlobalOrArgAttr(StratifiedAttrs Attr) {
+  return Attr.reset(AttrEscapedIndex).reset(AttrUnknownIndex).any();
+}
+
+static Optional<StratifiedAttr> valueToAttr(Value *Val) {
   if (isa<GlobalValue>(Val))
-    return AttrGlobalIndex;
+    return AttrGlobal;
 
   if (auto *Arg = dyn_cast<Argument>(Val))
     // Only pointer arguments should have the argument attribute,
     // because things can't escape through scalars without us seeing a
     // cast, and thus, interaction with them doesn't matter.
     if (!Arg->hasNoAliasAttr() && Arg->getType()->isPointerTy())
-      return argNumberToAttrIndex(Arg->getArgNo());
+      return argNumberToAttr(Arg->getArgNo());
   return None;
 }
 
-static StratifiedAttr argNumberToAttrIndex(unsigned ArgNum) {
+static StratifiedAttr argNumberToAttr(unsigned ArgNum) {
   if (ArgNum >= AttrMaxNumArgs)
-    return AttrAllIndex;
-  return ArgNum + AttrFirstArgIndex;
+    return AttrUnknown;
+  return 1 << (ArgNum + AttrFirstArgIndex);
 }
 
 static EdgeType flipWeight(EdgeType Initial) {
   switch (Initial) {
   case EdgeType::Assign:
     return EdgeType::Assign;
   case EdgeType::Dereference:
     return EdgeType::Reference;
for (auto &Pair : Map) {
     auto InitialNode = Pair.second;
     Worklist.push_back(InitialNode);
     while (!Worklist.empty()) {
       auto Node = Worklist.pop_back_val();
       auto *CurValue = findValueOrDie(Node);
       if (canSkipAddingToSets(CurValue))
         continue;
 
-      Optional<StratifiedAttr> MaybeCurIndex = valueToAttrIndex(CurValue);
-      if (MaybeCurIndex)
-        Builder.noteAttributes(CurValue, *MaybeCurIndex);
+      Optional<StratifiedAttr> MaybeCurAttr = valueToAttr(CurValue);
+      if (MaybeCurAttr)
+        Builder.noteAttributes(CurValue, *MaybeCurAttr);
 
       for (const auto &EdgeTuple : Graph.edgesFor(Node)) {
         auto Weight = std::get<0>(EdgeTuple);
         auto Label = Weight.first;
         auto &OtherNode = std::get<1>(EdgeTuple);
         auto *OtherValue = findValueOrDie(OtherNode);
 
         if (canSkipAddingToSets(OtherValue))
           continue;
 
         bool Added;
         switch (directionOfEdgeType(Label)) {
         case Level::Above:
           Added = Builder.addAbove(CurValue, OtherValue);
           break;
         case Level::Below:
           Added = Builder.addBelow(CurValue, OtherValue);
           break;
         case Level::Same:
           Added = Builder.addWith(CurValue, OtherValue);
           break;
         }
 
         auto Aliasing = Weight.second;
-        if (MaybeCurIndex)
-          Aliasing.set(*MaybeCurIndex);
-        if (auto MaybeOtherIndex = valueToAttrIndex(OtherValue))
-          Aliasing.set(*MaybeOtherIndex);
+        if (MaybeCurAttr)
+          Aliasing |= *MaybeCurAttr;
+        if (auto MaybeOtherAttr = valueToAttr(OtherValue))
+          Aliasing |= *MaybeOtherAttr;
         Builder.noteAttributes(CurValue, Aliasing);
         Builder.noteAttributes(OtherValue, Aliasing);
 
         if (Added)
           Worklist.push_back(OtherNode);
       }
     }
   }
 
   // There are times when we end up with parameters not in our graph (i.e. if
   // it's only used as the condition of a branch). Other bits of code depend on
   // things that were present during construction being present in the graph.
   // So, we add all present arguments here.
   for (auto &Arg : Fn->args()) {
     if (!Builder.add(&Arg))
       continue;
 
-    auto Attrs = valueToAttrIndex(&Arg);
-    if (Attrs.hasValue())
-      Builder.noteAttributes(&Arg, *Attrs);
+    auto Attr = valueToAttr(&Arg);
+    if (Attr.hasValue())
+      Builder.noteAttributes(&Arg, *Attr);
   }
 
   return FunctionInfo(Builder.build(), std::move(ReturnedValues));
 }
 
 void CFLAAResult::scan(Function *Fn) {
   auto InsertPair = Cache.insert(std::make_pair(Fn, Optional<FunctionInfo>()));
   (void)InsertPair;
AliasResult CFLAAResult::query(const MemoryLocation &LocA,
   if (!MaybeB.hasValue())
     return MayAlias;
 
   auto SetA = *MaybeA;
   auto SetB = *MaybeB;
   auto AttrsA = Sets.getLink(SetA.Index).Attrs;
   auto AttrsB = Sets.getLink(SetB.Index).Attrs;
 
-  // Stratified set attributes are used as markets to signify whether a member
-  // of a StratifiedSet (or a member of a set above the current set) has
-  // interacted with either arguments or globals. "Interacted with" meaning its
-  // value may be different depending on the value of an argument or global. The
-  // thought behind this is that, because arguments and globals may alias each
-  // other, if AttrsA and AttrsB have touched args/globals, we must
-  // conservatively say that they alias. However, if at least one of the sets
-  // has no values that could legally be altered by changing the value of an
-  // argument or global, then we don't have to be as conservative.
-  if (AttrsA.any() && AttrsB.any())
+  // If both values are local (meaning the corresponding set has attribute
+  // AttrNone or AttrEscaped), then we know that CFLAA fully models them: they
+  // may-alias each other if and only if they are in the same set
+  // If at least one value is non-local (meaning it either is global/argument or
+  // it comes from unknown sources like integer cast), the situation becomes a
+  // bit more interesting. We follow three general rules described below:
+  // - Non-local values may alias each other
+  // - AttrNone values do not alias any non-local values
+  // - AttrEscaped values do not alias globals/arguments, but they may alias
+  // AttrUnknown values
+  if (SetA.Index == SetB.Index)
     return MayAlias;
-
-  // We currently unify things even if the accesses to them may not be in
-  // bounds, so we can't return partial alias here because we don't know whether
-  // the pointer is really within the object or not.
-  // e.g. Given an out of bounds GEP and an alloca'd pointer, we may unify the
-  // two. We can't return partial alias for this case. Since we do not currently
-  // track enough information to differentiate.
-  return SetA.Index == SetB.Index ? MayAlias : NoAlias;
+  if (AttrsA.none() || AttrsB.none())
+    return NoAlias;
+  if (AttrsA.test(AttrUnknownIndex) || AttrsB.test(AttrUnknownIndex))
+    return MayAlias;
+  if (isGlobalOrArgAttr(AttrsA) && isGlobalOrArgAttr(AttrsB))
+    return MayAlias;
+  return NoAlias;
 }
 
 char CFLAA::PassID;
 
 CFLAAResult CFLAA::run(Function &F, AnalysisManager<Function> &AM) {
   return CFLAAResult(AM.getResult<TargetLibraryAnalysis>(F));
 }
 

llvm/trunk/lib/Analysis/StratifiedSets.h

StratifiedIndex getAbove() const {
       return Link.Above;
     }
 
     StratifiedAttrs &getAttrs() {
       assert(!isRemapped());
       return Link.Attrs;
     }
 
-    void setAttr(unsigned index) {
-      assert(!isRemapped());
-      assert(index < NumStratifiedAttrs);
-      Link.Attrs.set(index);
-    }
-
     void setAttrs(const StratifiedAttrs &other) {
       assert(!isRemapped());
       Link.Attrs |= other;
     }
 
     bool isRemapped() const { return Remap != StratifiedLink::SetSentinel; }
 
     /// For initial remapping to another set
public:
   }
 
   bool addWith(const T &Main, const T &ToAdd) {
     assert(has(Main));
     auto MainIndex = *indexOf(Main);
     return addAtMerging(ToAdd, MainIndex);
   }
 
-  void noteAttribute(const T &Main, unsigned AttrNum) {
-    assert(has(Main));
-    assert(AttrNum < StratifiedLink::SetSentinel);
-    auto *Info = *get(Main);
-    auto &Link = linksAt(Info->Index);
-    Link.setAttr(AttrNum);
-  }
-
   void noteAttributes(const T &Main, const StratifiedAttrs &NewAttrs) {
     assert(has(Main));
     auto *Info = *get(Main);
     auto &Link = linksAt(Info->Index);
     Link.setAttrs(NewAttrs);
   }
 
   StratifiedAttrs getAttributes(const T &Main) {

llvm/trunk/test/Analysis/CFLAliasAnalysis/attr-escape.ll

+; This testcase ensures that CFL AA handles escaped values no more conservative than it should
+
+; RUN: opt < %s -disable-basicaa -cfl-aa -aa-eval -print-all-alias-modref-info -disable-output 2>&1 | FileCheck %s
+; RUN: opt < %s -aa-pipeline=cfl-aa -passes=aa-eval -print-all-alias-modref-info -disable-output 2>&1 | FileCheck %s
+
+; CHECK: Function: escape_ptrtoint
+; CHECK: NoAlias: i32* %a, i32* %x
+; CHECK: NoAlias: i32* %b, i32* %x
+; CHECK: NoAlias: i32* %a, i32* %b
+; CHECK: MayAlias: i32* %a, i32* %aAlias
+; CHECK: NoAlias: i32* %aAlias, i32* %b
+define void @escape_ptrtoint(i32* %x) {
+  %a = alloca i32, align 4
+  %b = alloca i32, align 4
+  %aint = ptrtoint i32* %a to i64
+  %aAlias = inttoptr i64 %aint to i32*
+  ret void
+}