[PATCH] D62362: [COFF] De-virtualize Chunk and SectionChunk

[Alexandre Ganea via Phabricator via llvm-commits]

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