[PATCH] D26831: [LangRef] Update the TBAA section

[Anna Thomas via llvm-commits]

anna added inline comments.


================
Comment at: docs/LangRef.rst:4438
+tuples of names (these are strings) and pointers to a parent node which can
+either be the TBAA root or another scalar type descriptor.  All scalar type
+descriptors within a given TBAA root form a tree via this parent link --
----------------
I'm confused here. By definition, scalar type descriptors don't contain other types. So, how can it have another scalar type descriptor as a parent? 





================
Comment at: docs/LangRef.rst:4452
+*Access tags* are metadata nodes attached to load and store instructions.
+Access tags use type descriptors to use describe the *location* being
+accessed in terms of the type system of the higher level language.  Access
----------------
Nit: Extra word 'use'.
use type descriptors to describe the *location*


================
Comment at: docs/LangRef.rst:4494-4496
+structured in a way that for every access tag ``(BaseTy, AccessTy,
+Offset)``, ``GetAliasingLocations((BaseTy, AccessTy, Offset))`` contains
+``(AccessTy, AccessTy, 0)``.
----------------
Could you please explain why this is needed? 


================
Comment at: docs/LangRef.rst:4537
+
+    CharScalarTy = ("char", Root)
+    FloatScalarTy = ("float", CharScalarTy)
----------------
Is `Root` an identifier in TBAA?


================
Comment at: docs/LangRef.rst:4546-4547
 
-    !0 = !{ !"an example type tree" }
-    !1 = !{ !"int", !0 }
-    !2 = !{ !"float", !0 }
-    !3 = !{ !"const float", !2, i64 1 }
-
-The first field is an identity field. It can be any value, usually a
-metadata string, which uniquely identifies the type. The most important
-name in the tree is the name of the root node. Two trees with different
-root node names are entirely disjoint, even if they have leaves with
-common names.
-
-The second field identifies the type's parent node in the tree, or is
-null or omitted for a root node. A type is considered to alias all of
-its descendants and all of its ancestors in the tree. Also, a type is
-considered to alias all types in other trees, so that bitcode produced
-from multiple front-ends is handled conservatively.
-
-If the third field is present, it's an integer which if equal to 1
-indicates that the type is "constant" (meaning
+``GetAliasingLocations(tag3)`` is ``{tag3, tag4, tag7, (CharScalarTy,
+IntScalarTy, 0)}``.  This means that ,other than ``tag3`` itself, ``tag3``
+aliases ``tag4``, ``tag7`` and ``tag8``, reflecting the fact that
----------------
The mandatory `(AccessTy, AccessTy, 0)` for the result of `GetAliasingLocations(tag3)`refers to `(CharScalarTy, IntScalarTy, 0)` right? 

Can we have more than 2 `AccessTy`?


================
Comment at: docs/LangRef.rst:4547
+``GetAliasingLocations(tag3)`` is ``{tag3, tag4, tag7, (CharScalarTy,
+IntScalarTy, 0)}``.  This means that ,other than ``tag3`` itself, ``tag3``
+aliases ``tag4``, ``tag7`` and ``tag8``, reflecting the fact that
----------------
Nit: extra comma? 'This means that other than ``tag3`` itself,'


================
Comment at: docs/LangRef.rst:4555-4556
+
+Finally, access tags can also optionally indicate that the memory access is
+being done on an invariant memory location.
+
----------------
Do we need this here? The representation of `access tags` explains it in detail.


================
Comment at: docs/LangRef.rst:4575-4576
+Struct type descriptors are represented as ``MDNode`` s with an odd number
+of operands greater than 1.  The first operand is an ``MDString`` which,
+like for scalar type descriptors, is for ergonomics -- LLVM only cares that
+it finds an ``MDString`` there.  After this the name, we struct type
----------------
Could we show the MDString argument for the struct type descriptors in the type system descriptor graph in the above example:
```
InnerStructTy = {"InnerStructType", (IntScalarTy, 0), (FloatScalarTy, 4)}
```


================
Comment at: docs/LangRef.rst:4577
+like for scalar type descriptors, is for ergonomics -- LLVM only cares that
+it finds an ``MDString`` there.  After this the name, we struct type
+descriptors have a sequence of alternating ``MDNode`` and ``ConstantInt``
----------------
'After this name, the struct type...'


https://reviews.llvm.org/D26831