[PATCH] D26635: [TBAA] Don't generate invalid TBAA when merging nodes

[Sanjoy Das via llvm-commits]

sanjoy added inline comments.


================
Comment at: lib/Analysis/TypeBasedAliasAnalysis.cpp:462
+  // the root node.  In either case, we don't have any useful TBAA
+  // metadata to attach.
+  if (!Ret || Ret->getNumOperands() < 2)
----------------
manmanren wrote:
> When the only common base is the root node, is it illegal to generate a tag based on the root?
The comment at the top of the file states:

```
The second field is the access type node, it
// must be a scalar type node.
```


================
Comment at: unittests/Analysis/TBAATest.cpp:75
+  auto *RootMD = MD.createTBAARoot("tbaa-root");
+  auto *MD1 = MD.createTBAANode("scalar-a", RootMD);
+  auto *StructTag1 = MD.createTBAAStructTagNode(MD1, MD1, 0);
----------------
manmanren wrote:
> Why are we testing the old scalar TBAA here with createTBAANode?
The `OldTBAATest` name is misleading here, I'll change it to be more generic.  However, the only thing we've deprecated and removed support is the using scalar tbaa *tags*.  So far I don't see anything in the TBAA implementation that indicates we don't support scalar TBAA *nodes*.  For instance, see line 247:

```
    // Fast path for a scalar type node and a struct type node with a single
    // field.
    if (Node->getNumOperands() <= 3) {
      uint64_t Cur = Node->getNumOperands() == 2
                         ? 0
                         : mdconst::extract<ConstantInt>(Node->getOperand(2))
                               ->getZExtValue();
      Offset -= Cur;
      MDNode *P = dyn_cast_or_null<MDNode>(Node->getOperand(1));
      if (!P)
        return TBAAStructTypeNode();
      return TBAAStructTypeNode(P);
    }
```

Even if removing scalar TBAA nodes completely is going to ultimately be a better final state (I think it will be), the amount of work is non-trivial I'd rather not fold in all of that work in this cleanup work I'm currently doing.


https://reviews.llvm.org/D26635