[PATCH] D26438: [Verifier] Add verification for TBAA metadata

[Manman Ren via llvm-commits]

manmanren added a comment.

> There is one place where there is an ambiguity in the specification:
>  given a base type !{!"whatever", !N, i64 1} there is no way to tell if
>  this is a struct containing !N at offset i64 1 or a scalar immutable
>  TBAA node with !N as the parent. Right now I interpret this as the
>  former (as the implementation has been doing for 3 years). I will fix
>  the specification in a forthcoming documentation patch.

I may have misunderstood your point, but for struct-path TBAA, the last integer for a type node is the offset. Only for old scalar TBAA, the last optional integer specifies whether it is immutable. There is no ambiguity in struct-path TBAA.

This patch is for general goodness. Thanks for working on it. I wonder if it makes sense to share some logic between the verifier and TypeBasedAliasAnalysis.cpp, in terms of how to parse the various fields of the struct path tag nodes and the struct type nodes.

Cheers,
Manman



================
Comment at: lib/IR/Verifier.cpp:409
   void visitBasicBlock(BasicBlock &BB);
   void visitRangeMetadata(Instruction& I, MDNode* Range, Type* Ty);
+  void visitDereferenceableMetadata(Instruction &I, MDNode *MD);
----------------
Can we also change the format here, like the below statement?


================
Comment at: lib/IR/Verifier.cpp:4050
+    visitTBAAMetadata(I, TBAA);
+  }
+
----------------
I wonder why we switched the order between TBAAMetadata and MD_dereferenceable_or_null.


https://reviews.llvm.org/D26438