[PATCH] D133996: Add a cache for DL.getTypeAllocSize() to BasicAA.

[Justin Lebar via Phabricator via llvm-commits]

jlebar added a comment.

> I think a cache could generally make sense, though I wonder why it is BasicAA specific, and not part of DataLayout itself?

I was hesitant to add a cache on DataLayout because

- it's of unbounded size and lifetime (though I guess every Type that's created also permanently adds some memory usage in the LLVM context, so maybe this isn't an issue?).  Whereas in here the lifetime is bounded by the lifetime of the BasicAAResult.
- in DataLayout, it's hard to justify why we cache getTypeAllocSize but not any of the other properties.
- this is clearly a win in BasicAA (in my benchmark) but like, who knows how people use DataLayout, maybe it's not a win for all ways one could use it.

That said I don't feel strongly!  WDYT?

> I would also be interested in whether the GEPs in your case are mostly constant offset GEPs. It's on my roadmap to canonicalize those to i8 GEPs, which would allow us to save on a lot of redundant offset calculation in both BasicAA and other places.

In my case I don't think they are.  This is an XLA benchmark, so we tend to have 4D pointers where one or a few offsets are variable.  Though I agree that canonicalization would help if they were all constant.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133996