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

[Manman Ren via llvm-commits]

manmanren 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)
----------------
sanjoy wrote:
> 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.
> ```
I see. I was wondering if it caused any compilation failure or miscompile.



================
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);
----------------
sanjoy wrote:
> 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.
We have createTBAAScalarTypeNode and it is used by clang currently to generate a scalar type node.
createTBAANode was used by clang with the old TBAA scalar system.


https://reviews.llvm.org/D26635