[PATCH] D93654: [TableGen] Change getAllDerivedDefinitions() to return an ArrayRef

[David Blaikie via Phabricator via llvm-commits]

dblaikie added a comment.

In D93654#2470564 <https://reviews.llvm.org/D93654#2470564>, @Paul-C-Anagnostopoulos wrote:

> I'm not yet clever enough to do profiling.

Happy to help with anything in that regard. I don't often do execution profiling, more often memory profiling, but have some rough knowledge/manage to stick it all together from time to time.

> It just seemed like building large record vectors multiple times was a waste of time. Perhaps I embarked on this change without sufficient evidence of its necessity.

Yeah, this follow-on change makes sense to me if the initial change does, but that initial change to add caching seems like it should be justified with a profile or the like - otherwise it's hard to know if they're big enough for the copies to matter, etc. (& adding a cache can increase memory usage - which might have other knock-on effects, etc)

> I will move the copy comments above the definition.

Great!


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

https://reviews.llvm.org/D93654