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

[David Blaikie via Phabricator via llvm-commits]

dblaikie added a comment.

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

> Ah yes, good point. I was concerned about time, not memory.
>
> Hmm. Perhaps I should abandon this. What do people think?

I'd say at least put it on hold/revisit the caching addition - sounds like you're generally interested in improving the performance of TableGen, so working on getting good profile results to motivate not just this change but others you might find, etc seems like a good place to start. If it turns out the caching's good, you can revisit this patch. (but I guess from a Phabricator patch status - "abandoned" might be an option, I guess you can always unabandon it if things go this way - your call)

You don't have to immediately revert the caching to begin with - figure out the profiling, try measuring with tblgen as it is in-tree with the caching, and then try locally reverting/profiling and see if what the difference looks like. If there's something in it, great, otherwise you can revert the caching and see what might be the hotter parts that are worth profiling.


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

https://reviews.llvm.org/D93654