[NFC] Make the GCData constructor a move constructor #6946
Conversation
src/wasm-interpreter.h
Outdated
| auto allocation = std::make_shared<GCData>(type.getHeapType(), data); | ||
| // | ||
| // This consumes the input |data| entirely. | ||
| Literal makeGCData(Literals& data, Type type) { |
There was a problem hiding this comment.
We should change this to take Literals&& data to make it clearer in callers that data will be moved from.
There was a problem hiding this comment.
Oh right, good idea. Done.
| SmallVector(std::initializer_list<T> init) { | ||
| for (T item : init) { | ||
| push_back(item); | ||
| } | ||
| } | ||
| SmallVector(size_t initialSize) { resize(initialSize); } | ||
|
|
||
| SmallVector<T, N>& operator=(const SmallVector<T, N>& other) { |
There was a problem hiding this comment.
Should we add a move assignment operator for completeness?
There was a problem hiding this comment.
Hmm, perhaps let's wait until there is a user? Otherwise adding it now would be dead/untested code.
There was a problem hiding this comment.
The problem is that if someone does try to use move assignment, the compiler will gracefully fall back to using copy assignment and the user will have no idea that they didn't get the move they wanted. We can prevent that by providing a move assignment operator eagerly, and the code should be simple enough that there shouldn't be any problems. (Alternatively we could provide a move assignment whose body is a WASM_UNREACHABLE.)
There was a problem hiding this comment.
Good point, that does sound risky. Added.
This avoids creating a large Literals (SmallVector of Literal) and then copying it. All the places
that construct GCData do not need the Literals afterwards.
Also add a few other constructor type things, all as a result of C++ compiler errors after
the GCData constructor change.
This gives a 7% speedup on the
--precomputebenchmark from #6931