[llvm-commits][PATCH] elfobjectwriter patch (ARM/MC/ELF)

+llvmdev

Refactoring the x86 dependent code from ELFObjectWriter class has
repercussions among several (conflicting) axes of consideration,

1. namespace pollution - minimize pollution of the llvm: namespace
2. consistency - try to maintain as small a delta between the changes
3. linking - minimize the number of additional cross dependency
between the existing libraries
4. clarity - avoid special case switching as much as possible -
5. Xfactor - how clean is the overall resulting design?

The possible ways forward I see are

SMALL: keep the code nearly as is - place a switch inside
ELFObjectWriter::RecordRelocation and dispatch to
ELFObjectWriterImpl::RecordRelocation<ARCH>
1. +1 no new classes
2. +1 tiny patch
3. +1 no new classes, just one additional function so far.
4. -2 need to have special case switching for every routine that needs
to be tweaked.
5. -2 Terrible! So far, its just one new switch, but ...

tryA: move the functionality of the ELFObjectWriterImpl class into
ELFObjectWriter, and subclass ELFObjectWriter to
<ARCH>ELFObjectWriter.
Change most ELF specific routines to be virtual - except for the low
level Write* routines -
1. -1 at least new classes ARMELFObjectWriter and X86ELFObjectWriter
2. -1 large patch
3. +2 Resulting special cases are isolated in their own class
4. +1 Depends upon virtual dispatch for higher level differentiation
- removes unnecessary trampoline between ELFObjectWriter and
ELFObjectWriterImpl
5. +2 This approach is the best in terms of the resulting design. The
only drawback is the distinction between MachO and ELF

This looks OK to me, and is similar to what I'm looking at for MachO. I confess I'm not completely clear on why the ELF writer is currently split into the Writer and WriterImpl classes, so that may be worth answering before proceeding too far.

Thanks for the feedback, Jim. I'll wait a bit to see if there's an
interesting answer to:
Anyone have any idea why the ELFObjectWriter/ and WriterImpl classes
were split?