[llvm-commits] [pr12979] updated patch

[Nuno Lopes]

> I have change the tbaa merging logic to select the lowest common
> ancestor and reorganized the tests a bit.

The patch looks great overall.
Just a few comments:

+  while (T) {
+    PathA.push_back(T);
+    T = T->getNumOperands() == 2 ? cast_or_null<MDNode>(T->getOperand(1)) : 
0;

should be >= 2 there, since TBAA metadata can have 3 arguments. Same thing 
in the 2nd loop.
Also, probably it's a good idea to have a  fast exit if A==B.
A few comments in the range merging function would be nice :)  e.g. in the 
loop to merge the ranges in the two ends, etc.  The whole algorithm is 
non-trivial (to me, at least).


Without wanting to delay your patch much farther, I would like to insist in 
my previous idea on the ConstantRangeUnion ADT (or whatever name you like).
What do you think about the following API:
- range metadata -> ConstantRangeUnion
- ConstantRangeUnion -> range metadata
- ConstantRangeUnion:  union, intersection w/ ConstantRangeUnion and 
ContantRange
(and in the future, add support for all the other operations that 
ConstantRange supports)

I can see use of this API here, and in the value propagation pass, for 
example. I can certainly help (w/ code).

Nuno