[PATCH] D41501: [Analysis] Support aggregate access types in TBAA

[Hal Finkel via Phabricator via llvm-commits]

hfinkel accepted this revision.
hfinkel added a comment.
This revision is now accepted and ready to land.

A couple comments, but otherwise LGTM.



================
Comment at: lib/Analysis/TypeBasedAliasAnalysis.cpp:640
+  if (NewFormat) {
+    TBAAStructTypeNode BaseType(BaseTag.getAccessType());
+    TBAAStructTypeNode FieldType(SubobjectTag.getBaseType());
----------------
Please name this something other than `BaseType` because we already have a variable by that name in scope, and moreover, this is an access type (and so calling it 'BaseType' seems somewhat confusing (given that `BaseType` above is not necessarily an access type). Maybe just calling it BaseAccessType would be better.

Alternatively, just remove this because, to get here, I think that BaseType is always equal to the access type.


================
Comment at: lib/Analysis/TypeBasedAliasAnalysis.cpp:642
+    TBAAStructTypeNode FieldType(SubobjectTag.getBaseType());
+    if (hasField(BaseType, FieldType)) {
+      if (GenericTag)
----------------
Especially because, in the past, access types were only scalars, I think it would be useful to have a comment here reminding the reader than the access type might be an aggregate (thus explaining the need for this check).


https://reviews.llvm.org/D41501