[llvm-commits] [llvm] r119149 - /llvm/trunk/lib/MC/ELFObjectWriter.cpp

[Jason Kim]

On Mon, Nov 15, 2010 at 8:52 AM, Rafael Espíndola
<rafael.espindola at gmail.com> wrote:
> On 15 November 2010 11:18, Jason W Kim <jason.w.kim.2009 at gmail.com> wrote:
>> Author: jasonwkim
>> Date: Mon Nov 15 10:18:39 2010
>> New Revision: 119149
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=119149&view=rev
>> Log:
>> Dovetail with Dan Dunbar's rework of ELFObjectWriter.
>> Added 2 new subclasses - X86ELFObjectWriter and ARMELFObectWriter.
>> ARM and X86 require different code for RecordRelocation(), possibly others.
>
> Thanks!
>
> Only comment is that it is probably better to make methods virtual as needed.

Hmm... If the perf hit becomes measurable, we can pare down the
virtuals once more ELF targets become MC'ized. Right now, mixing
nonvirtuals with virtuals for large methods is probably more confusing
than not (might lead to subtle bugs too). Besides, MCObjectWriter uses
nonvirtuals only for small leaf methods and virtual for everything
else, so keeping that logic consistent is good too, I guess.

>
> One day we might want to move RecordRelocation to lib/Target, but for
> now it is better to keep it here so that we see any potential for code
> sharing.

I initially tried this, but the result was a link issue due to library
order not being correct. Meh. Let's punt on it for now. I agree.

>
> Cheers,
> Rafael
>

Thanks!!
-jason