[llvm-commits] RuntimeDyld RelocationResolver
dmalyshev at accesssoftek.com
Tue Oct 11 16:29:26 CDT 2011
Thank you for review. The changed patch is attached.
> Target names for classes like this are generally done as a prefix, not
> a suffix. It would be better to do that here for consistency.
> x86 vs. x86_64?
Both. I have added the explanation in the attached patch.
> This seems odd? Placeholder? If so, a FIXME to that effect would be good.
> LLVM doesn't generally use right-justified comments like this. Not a
> big deal, but it looks a bit odd in context.
> For now, though, it'd be good to split the target bits into separate
> files rather than keeping it all together in one.
> This seems odd. At minimum, copious comments are needed explaining
> what's going on here.
> We shouldn't need to do by-name lookup here. Mapping names should have
> been handled by the loader when the symbol table was processed.
> Everything at this level should be able to use symbol table indices.
> No braces when there's just one statement.
> Not all relocations are on functions. How are global values handled,
> for example?
This part of code runs only for branch relocations and will not be executed for global values.
> Is this target memory? The local memory where our copy is stored? In
> any case, addr/length is probably better than start/end so we can
> handle symbols w/o any data associated (aliases).
I have added the explanation in the attached patch.
> Relocations are inherently target and platform specific. ARM MachO vs.
> ARM ELF are very different, for example. This appears to be trying for
> a one-size-fits-all solution. I don't think that's going to work out.
> This is the biggest concern I have with this patch. The relocations
> should use the definitions in the Object and ELF/MachO file format
> headers and handle them distinctly.
You are right. Originally, relocations are inherently target and platform specific.
Though, emitter emits the relocations to the target memory on some unified manner, so once in the target memory they are target specific only (i.e. ARM specific, since ARM MachO vs. ARM ELF differences are handled).
Relocation resolver dials with already emitted relocations, and depdends on target only because of that.
-------------- next part --------------
A non-text attachment was scrubbed...
Size: 15437 bytes
Url : http://lists.cs.uiuc.edu/pipermail/llvm-commits/attachments/20111011/8fab1934/attachment-0001.obj
More information about the llvm-commits