The two methods don't belong in BinaryFunction methods.
Move the dispatch tables into target-specific MCPlusBuilder methods.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
Plan to break addRelocation into the target-specific check in MCPlusBuilder and actually adding the relocation to the container in BinaryFunction.
bolt/include/bolt/Core/MCPlusBuilder.h | ||
---|---|---|
419 | I'm a bit confused with the name, for example, what does it mean to have a supported relocation? Why is ELF::R_AARCH64_CALL26 unsupported? I think either the comments here need to be expanded a bit or the function get a better name, because this "support" is very specific to the needs of BinaryFunction. |
bolt/include/bolt/Core/MCPlusBuilder.h | ||
---|---|---|
419 | What name and comment would you suggest? These are relocations we store for function splitting and reordering. |
bolt/include/bolt/Core/MCPlusBuilder.h | ||
---|---|---|
419 | I don't have good names that give the necessary context. I guess something like "shouldRecordCodeReloc/shouldStoreReloc" or "shouldRetainCodeReloc". The context might go in a comment maybe... the context is that these relocations are essentially used by our disassembly step to better understand code (by using these code relocs). For ARM, they help us decode instruction operands unambiguously, but sometimes we might discard them because we already have the necessary information in the instruction itself (for example, we don't need to record CALL relocs in ARM because we can fully decode the target from the call operand). For X86, they might be used in scanExternalRefs when we want to skip a function but still patch references inside it. |
I'm a bit confused with the name, for example, what does it mean to have a supported relocation? Why is ELF::R_AARCH64_CALL26 unsupported? I think either the comments here need to be expanded a bit or the function get a better name, because this "support" is very specific to the needs of BinaryFunction.