[PATCH] D99851: [SROA][TBAA] Handle shift of regular TBAA nodes

[William Moses via Phabricator via llvm-commits]

wsmoses added a comment.

I'm not quite sure I understand what you mean @jeroen.dobbelaere by an i64 store of a long being split to 2 i32 int stores. I presume you mean that there was originally a struct {int, int} that is currently using an i64 store to do both at the same time. I don't believe that a "long" store would have this apply since its tbaa would simply be one long, right? Moreover, this shifting is only in use in SROA at the moment so I'm not sure it would split a store of a non-aggregate type (such as an i64).

I shall update the test case. Without the patch, in my testing verifier fails, but the actual TBAA nodes should be actually defined in the check's.

My initial concern with the mechanism for adding the offset is that naiively adding the offset here may be legal as it appears that TBAA nodes must have an offset corresponding to an offset defined in the base node. This may make it possible to perform a legal shift that does not have a defined offset within the base type (and in practice on the test suite, this appeared to occur).

I'd be happy to take a further look to try to examine this but won't be able to start diving deeply in for at least another week (several imminent conference paper deadlines). If someone else would like to take over this in the interim, feel free. It sounded from @aheejin that this was somewhat time sensitive.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D99851/new/

https://reviews.llvm.org/D99851