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

[Jason Kim]

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

tryB: subclass ELFObjectWriterImpl instead - I am still working out
the details on this one - but as of right now, it is just as complex
as the tryA case.
The only benefit to this is approach is the superficial similarity
between the ELFObjectWriter and the MachObjectWriter - in that both
still trampoline into an *Impl class to do the actual work.
Unfortunately, it also adds a requirement for registering a NEW
<ARCH>ELFObjecWriterImpl class (i.e. in addition to the existing
createELFObjectWriter, and without namespace pollution to llvm,
creates a linkage dependency failure).
I will reply to this thread with a patch as soon as I finish this variant.

There are several attachments:

small:
 small-elfwriter-cpp (application/octet-stream) 2K
 small-elfwriter-rename-record (application/octet-stream) 2K

tryA
  arm-mc-elf-s07-elfwriter-tryA-combined.patch (text/x-patch) 43K -
this is the combined patch - In order to make it more clear, I broke
up the steps into a dozen or so smaller patches.
  tryA.tgz  - archive of patches
  They are combined into a tar archive - the README is reproduced here.

Thanks for reading.
-jason
-------------- next part --------------
A non-text attachment was scrubbed...
Name: arm-mc-elf-s07-elfwriter-tryA-combined.patch
Type: text/x-patch
Size: 43386 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20101110/bf1b0e5e/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: small-elfwriter-cpp
Type: application/octet-stream
Size: 1557 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20101110/bf1b0e5e/attachment.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: small-elfwriter-rename-record
Type: application/octet-stream
Size: 1773 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20101110/bf1b0e5e/attachment-0001.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: tryA.tgz
Type: application/x-gzip
Size: 10036 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20101110/bf1b0e5e/attachment-0001.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: README
Type: application/octet-stream
Size: 1287 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20101110/bf1b0e5e/attachment-0002.obj>