[PATCH] D67305: [AliasSetTracker] Update AAInfo check.

[Alina Sbirlea via Phabricator via llvm-commits]

asbirlea marked an inline comment as done.
asbirlea added inline comments.


================
Comment at: include/llvm/Analysis/AliasSetTracker.h:88
         AAInfo = NewAAInfo;
-      else {
-        AAMDNodes Intersection(AAInfo.intersect(NewAAInfo));
-        if (!Intersection.TBAA || !Intersection.Scope ||
-            !Intersection.NoAlias) {
-          // NewAAInfo conflicts with AAInfo.
+      else if (AAInfo.TBAA != NewAAInfo.TBAA ||
+            AAInfo.Scope != NewAAInfo.Scope ||
----------------
hfinkel wrote:
> This seems overly restrictive. The intersection might return a valid combination of the two metadata nodes (as a conservative combination). I thought that it only returned null if there's no valid way to combine them.
The intersection method does this:
```
    AAMDNodes Result;
    Result.TBAA = Other.TBAA == TBAA ? TBAA : nullptr;
    Result.Scope = Other.Scope == Scope ? Scope : nullptr;
    Result.NoAlias = Other.NoAlias == NoAlias ? NoAlias : nullptr;
    return Result;
```
This is its only callsite.
I don't really know what a valid combination of two metadata nodes is TBH.

The "before" state was "if all fields are null, use tombstonekey, otherwise, use above intersection (fields set to null if different)". This is incomplete because any different field may need to merge sets (`return SizeChanged = true;`).

After I amended the above (current state), now we may return tombstonekey even when a field was nullptr to begin with in both `AAInfo` and `NewAAInfo`. Hence the change below to check for any difference.

Could you help me out with suggesting a less restrictive check?



Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67305/new/

https://reviews.llvm.org/D67305