llvm dev - Nov 2010 - [LLVMdev] MC ELFObjectWriter backend refactoring

I have been working on getting ELF object file writing working for the MBlaze
backend. Currently, each supported backend calls
ELFObjectWriter::createELFObjectWriter from within the backend's
TargetAsmBackend::createObjectWriter method. The createELFObjectWriter method
then creates a new backend specific ELFObjectWriter class (either
X86ELFObjectWriter or ARMELFObjectWriter) by decoding a given machine type.

The attached patch does some code refactoring so that a backend can create
it's own ELFObjectWriter class and instantiate this class directly in
TargetAsmBackend::createObjectWriter. This allows backend specific stuff in
lib/MC/ELFObjectWriter.cpp to be moved to the backend's directory.

There are obviously several things that could be done differently:
1. The backend specific ELFObjectWriter classes are declared and implemented in
X86AsmBackend.cpp and ARMAsmBackend.cpp; it may be better to put these classes
in their own files.

2. The ELFObjectWriter base class is declared in llvm/MC/MCObjectWriter.h; it
may be better to put this declaration in its own header file.

3. Some of the flags, such as Is64Bit and EMachine, could be virtual methods
instead of values passed to the base ELFObjectWriter class.

4. Refactoring of MachObjectWriter and WinCOFFObjectWriter could be done as
well.

5. I could just add the MBlaze backend specific stuff into ELFObjectWriter like
all of the other backends.

Are their any other opinions on this? I personally prefer all of the MBlaze
backend stuff to be contained inside of lib/Target/MBlaze so that I don't
have to hunt around the LLVM source tree when I'm implementing/fixing the
MBlaze backend. If #5 is preferred instead, then I can re-implement the MBlaze
ELF stuff that way instead.

--
Wesley Peck
University of Kansas
SLDG Laboratory


-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20101118/21ad2bf5/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: mcelf.patch
Type: application/octet-stream
Size: 45644 bytes
Desc: not available
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20101118/21ad2bf5/attachment.obj>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20101118/21ad2bf5/attachment-0001.html>

> 5. I could just add the MBlaze backend specific stuff into ELFObjectWriter
> like all of the other backends.
> Are their any other opinions on this? I personally prefer all of the MBlaze
> backend stuff to be contained inside of lib/Target/MBlaze so that I
don't
> have to hunt around the LLVM source tree when I'm implementing/fixing
the
> MBlaze backend. If #5 is preferred instead, then I can re-implement the
> MBlaze ELF stuff that way instead.
It probably depends on how much specific stuff you expect to be. If
the differences for ARM, X86, MBlaze, etc are just some constants and
the record relocation code, then it is probably better to have a
single ELF writer class that delegates to another class that does just
relocations and is implemented in the target directory.

Having the ELF writer class written in a header only looks like the
right solution if we do end up with per arch differences spread over
multiple methods.

Would you mind start by adding code to the existing file? Once we have
at least two working architectures we should have a better idea on how
to split it.
>
> --
> Wesley Peck
> University of Kansas
> SLDG Laboratory

Thanks,
Rafael

As a note, using the previous MC ELFObjectWriter patch, I have been able to get
initial ELF object file writing to work for the MBlaze backend without needing
to change the ELFObjectWrite.cpp file (beyond the previous patch). Right now it
can produce mostly identical ELF files compared to GCC for several examples. The
only difference seems to be the ordering of some of the relocations and the
omission of symbols that start with $ (I think this is on purpose).

The attached patch shows the changes that were necessary to get this working on
a new backend.

I'll hold off on committing these until someone has time to look over the
previous patch since they change things in the MC framework that I'm not too
familiar with.

--
Wesley Peck
University of Kansas
SLDG Laboratory


On Nov 18, 2010, at 10:40 AM, Wesley Peck wrote:
> I have been working on getting ELF object file writing working for the
MBlaze backend. Currently, each supported backend calls
ELFObjectWriter::createELFObjectWriter from within the backend's
TargetAsmBackend::createObjectWriter method. The createELFObjectWriter method
then creates a new backend specific ELFObjectWriter class (either
X86ELFObjectWriter or ARMELFObjectWriter) by decoding a given machine type.
> 
> The attached patch does some code refactoring so that a backend can create
it's own ELFObjectWriter class and instantiate this class directly in
TargetAsmBackend::createObjectWriter. This allows backend specific stuff in
lib/MC/ELFObjectWriter.cpp to be moved to the backend's directory.
> 
> There are obviously several things that could be done differently:
> 1. The backend specific ELFObjectWriter classes are declared and
implemented in X86AsmBackend.cpp and ARMAsmBackend.cpp; it may be better to put
these classes in their own files.
> 
> 2. The ELFObjectWriter base class is declared in llvm/MC/MCObjectWriter.h;
it may be better to put this declaration in its own header file.
> 
> 3. Some of the flags, such as Is64Bit and EMachine, could be virtual
methods instead of values passed to the base ELFObjectWriter class.
> 
> 4. Refactoring of MachObjectWriter and WinCOFFObjectWriter could be done as
well.
> 
> 5. I could just add the MBlaze backend specific stuff into ELFObjectWriter
like all of the other backends.
> 
> Are their any other opinions on this? I personally prefer all of the MBlaze
backend stuff to be contained inside of lib/Target/MBlaze so that I don't
have to hunt around the LLVM source tree when I'm implementing/fixing the
MBlaze backend. If #5 is preferred instead, then I can re-implement the MBlaze
ELF stuff that way instead.
> 
> --
> Wesley Peck
> University of Kansas
> SLDG Laboratory
> 
> <mcelf.patch>
> 
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20101119/433d4b04/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: mblaze-elf.patch
Type: application/octet-stream
Size: 13689 bytes
Desc: not available
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20101119/433d4b04/attachment.obj>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20101119/433d4b04/attachment-0001.html>

> Would you mind start by adding code to the existing file? Once we have
> at least two working architectures we should have a better idea on how
> to split it.
I can do that. As a note, I just sent another email (before I saw this one)
showing how much code was needed to add support for the MBlaze backend. Do you
want to take a look at that before I merge the MBlaze stuff into
ELFObjectWriter.c?

Thanks for reviewing this.

--
Wesley Peck
University of Kansas
SLDG Laboratory