[PATCH] [RuntimeDyld] Make ObjectBuffer/ObjectFile -> ObjectImage conversion a static method.

Rafael EspĂ­ndola rafael.espindola at gmail.com
Fri Mar 7 15:25:24 PST 2014


I don't know enough about the code to say much, but I agree that the
current virtual function calls look silly :-)

Please be sure to clang-format your patch.

On 5 March 2014 15:06, Lang Hames <lhames at gmail.com> wrote:
> Yep - the Obj local should go away. I'll make sure I do that if/when
> this gets committed.
>
> createObjectImageFromFile (and createObjectImage) should return
> unique_ptrs. I intend to change them in future work. (That's fairly
> orthogonal to the issues addressed in this patch, and I didn't want to
> complicate review).
>
> I kept loadObject's return value just to minimize the scope of this
> patch, but I agree that it should probably go away, since it's just
> returning its argument again.
>
> - Lang.
>
>
> On Wed, Mar 5, 2014 at 11:45 AM, David Blaikie <dblaikie at gmail.com> wrote:
>> I'm not sure I know enough about the actual code here to have high
>> level semantic suggestions, but one nit:
>>
>>   std::unique_ptr<ObjectImage> Obj(std::move(InputObject));
>>
>> seems unnecessary - you could just use the parameter directly (rename
>> it to "Obj" for example)
>>
>> At some point I guess createObjectImageFromFile will return a
>> unique_ptr? (so you don't have to call reset with the result in
>> RuntimeDyld::loadObject(ObjectFile*))
>>
>>
>> Curiosity:
>>
>> What does Dld->loadOObject(std::move(InputImage)) do? It's odd that it
>> takes an ObjectImage and returns one - does it return the same object?
>> Why not just have it take the thing by reference rather than taking
>> and returning ownership?
>>
>>
>> On Wed, Mar 5, 2014 at 11:20 AM, Lang Hames <lhames at gmail.com> wrote:
>>> Hi All,
>>>
>>> I would like to ensure that the target architecture for a
>>> RuntimeDyldImpl instance is known at construction time. Once this is
>>> possible, target specific logic (relocations/stub management) can be
>>> pushed down into the target specific subclasses, rather than living in
>>> large switch statements. This will improve both performance and
>>> readability.
>>>
>>> To support this work, this patch changes the way ObjectImages are
>>> constructed from ObjectFiles and ObjectBuffers: it replaces virtual
>>> methods on RuntimeDyldImpl with static methods on the relevant
>>> subclasses (RuntimeDyldELF, RuntimeDyldMachO). All the information
>>> needed to call the correct subclass method is already available at the
>>> call sites, so we don't lose any flexibility by switching to static
>>> methods here.
>>>
>>> Cheers,
>>> Lang.
>>>
>>> _______________________________________________
>>> llvm-commits mailing list
>>> llvm-commits at cs.uiuc.edu
>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>>



More information about the llvm-commits mailing list