[PATCH] D92674: [TableGen] Cache the vectors of records returned by getAllDerivedDefinitions().
Paul C. Anagnostopoulos via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Wed Dec 9 15:52:23 PST 2020
Paul-C-Anagnostopoulos added inline comments.
================
Comment at: llvm/lib/TableGen/Record.cpp:2600
+// the same vectors multiple times.
std::vector<Record *> RecordKeeper::getAllDerivedDefinitions(
+ StringRef ClassName) const {
----------------
craig.topper wrote:
> Paul-C-Anagnostopoulos wrote:
> > dblaikie wrote:
> > > Now that the result is cached, perhaps this function could return by const ref instead of by value, to avoid unnecessary copies of the resulting vector?
> > The vector must be copied, because there is nothing stopping the caller from modifying it.
> >
> > Do you think it's worth the effort to establish the rule that it cannot be modified and then investigate every call to see what's going on?
> >
> If the caller needs to modify, the caller can make a copy explicitly. I doubt most callers modify it. I think if you just change it to return const std::vector<Record*> & any code that does
>
> ```
> std::vector<Record*> Foo = getAllDerivedDefinitions
> ```
>
> will still compile. A copy will just be made by the assignment. This should cover most callers I would guess. Then we can through and individually change the callers to use "const std::vector &" to remove the copies.
>
>
That sounds like a good plan.
Some day I will get used to all the copying. I think.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D92674/new/
https://reviews.llvm.org/D92674
More information about the llvm-commits
mailing list