llvm dev - Oct 2015 - [elf2] How to support both big and little endian MIPS targets

Hi,

I need an advice how to support both big and little endian MIPS
targets in the new LLD ELF design and escape code duplication. MIPS
32-bit big and little endian targets are very similar and differ in
read/write functions. There are some options how to support both but I
cannot make the best choice.

1. Make MipsTargetInfo a class template. Use the template's argument
to select read/write routines.
2. Pass boolean argument to the MipsTargetInfo constructor. Use this
argument to select read/write routines.
3. Create a hierarchy of MipsTargetInfo classes. Keep common code in
the MipsTargetInfo class, move endian-dependent code into the
MipsELTargetInfo / MipsBETargetInfo descendants.
4. Anything else?

-- 
Simon Atanasyan

Choice 3 sounds too much, choices 1 or 2 seem better. I'd probably go with
1 because we already have lots of code which is templated for ELFT or
Is64Bits, and IsLE falls in the same category. Target dependent code
(Target.cpp) is small, so code bloat is not going to be an issue.

I'd define something like this: template <bool LE> class
MipsTargetInfo,
and use add32<LE>() instead of add32le and so on.

On Tue, Oct 13, 2015 at 3:17 AM, Simon Atanasyan <simon at atanasyan.com>
wrote:
> Hi,
>
> I need an advice how to support both big and little endian MIPS
> targets in the new LLD ELF design and escape code duplication. MIPS
> 32-bit big and little endian targets are very similar and differ in
> read/write functions. There are some options how to support both but I
> cannot make the best choice.
>
> 1. Make MipsTargetInfo a class template. Use the template's argument
> to select read/write routines.
> 2. Pass boolean argument to the MipsTargetInfo constructor. Use this
> argument to select read/write routines.
> 3. Create a hierarchy of MipsTargetInfo classes. Keep common code in
> the MipsTargetInfo class, move endian-dependent code into the
> MipsELTargetInfo / MipsBETargetInfo descendants.
> 4. Anything else?
>
> --
> Simon Atanasyan
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20151013/062df741/attachment.html>

Thanks for the advice. I've just sent the patch for review.

On Tue, Oct 13, 2015 at 4:45 PM, Rui Ueyama <ruiu at google.com>
wrote:
> Choice 3 sounds too much, choices 1 or 2 seem better. I'd probably go
with 1
> because we already have lots of code which is templated for ELFT or
> Is64Bits, and IsLE falls in the same category. Target dependent code
> (Target.cpp) is small, so code bloat is not going to be an issue.
>
> I'd define something like this: template <bool LE> class
MipsTargetInfo, and
> use add32<LE>() instead of add32le and so on.
>
> On Tue, Oct 13, 2015 at 3:17 AM, Simon Atanasyan <simon at
atanasyan.com>
> wrote:
>>
>> Hi,
>>
>> I need an advice how to support both big and little endian MIPS
>> targets in the new LLD ELF design and escape code duplication. MIPS
>> 32-bit big and little endian targets are very similar and differ in
>> read/write functions. There are some options how to support both but I
>> cannot make the best choice.
>>
>> 1. Make MipsTargetInfo a class template. Use the template's
argument
>> to select read/write routines.
>> 2. Pass boolean argument to the MipsTargetInfo constructor. Use this
>> argument to select read/write routines.
>> 3. Create a hierarchy of MipsTargetInfo classes. Keep common code in
>> the MipsTargetInfo class, move endian-dependent code into the
>> MipsELTargetInfo / MipsBETargetInfo descendants.
>> 4. Anything else?
-- 
Simon Atanasyan