[PATCH] D62362: [COFF] De-virtualize Chunk and SectionChunk
Alexandre Ganea via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Fri May 24 09:23:28 PDT 2019
aganea added inline comments.
================
Comment at: lld/COFF/Chunks.h:355
+
+inline size_t Chunk::getSize() const {
+ if (isa<SectionChunk>(this))
----------------
This is back to the future! C with manual vtables :) 64-bit pointers are a big problem for space. It'd be nice if we could optionally have 32-bit or even 16-bit pointers (yes it opens the door to other issues, but that could be catched with sanitizers or assertions). In many cases we pool objects of a given type or a given class hierarchy, so they are quite close to each other - having an option to copy the vtable close to the allocated pool would be nice to alleviate for issues like this. A 16-bit index would be more that enough in that case, in place of the 64-bit vtable.
================
Comment at: lld/COFF/Chunks.h:377
+
+inline bool Chunk::isHotPatchable() const {
+ if (isa<SectionChunk>(this))
----------------
Since you're doing this, I'm just wondering if the behavior for `isHotPatchable` shouldn't all be here? I find there's value having all related code together. Maybe there are other cases for the other "virtual-ish" functions below.
```
inline bool Chunk::isHotPatchable() const {
if (ChunkKind == SectionKind)
return File->HotPatchable;
return ChunkKind == ImportThunk; // add ImportThunk kind
}
```
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D62362/new/
https://reviews.llvm.org/D62362
More information about the llvm-commits
mailing list