This is an archive of the discontinued LLVM Phabricator instance.

[mips64] Add support for MCJIT for MIPS64
AbandonedPublic

Authored by vstefanovic on Oct 22 2014, 10:20 AM.

Details

Summary

Initial support for resolving MIPS64 relocations in MCJIT.

Diff Detail

Event Timeline

vstefanovic retitled this revision from to [mips64] Add support for MCJIT for MIPS64.
vstefanovic updated this object.
vstefanovic edited the test plan for this revision. (Show Details)
vstefanovic added reviewers: lhames, petarj.
vstefanovic set the repository for this revision to rL LLVM.
vstefanovic added a subscriber: Unknown Object (MLST).

This is the list of passing tests now:

LLVM :: ExecutionEngine/MCJIT/cross-module-sm-pic-a.ll
LLVM :: ExecutionEngine/MCJIT/eh-lg-pic.ll
LLVM :: ExecutionEngine/MCJIT/eh-sm-pic.ll
LLVM :: ExecutionEngine/MCJIT/hello-sm-pic.ll
LLVM :: ExecutionEngine/MCJIT/multi-module-sm-pic-a.ll
LLVM :: ExecutionEngine/MCJIT/non-extern-addend-smallcodemodel.ll
LLVM :: ExecutionEngine/MCJIT/remote/cross-module-sm-pic-a.ll
LLVM :: ExecutionEngine/MCJIT/remote/multi-module-sm-pic-a.ll
LLVM :: ExecutionEngine/MCJIT/remote/test-global-init-nonzero-sm-pic.ll
LLVM :: ExecutionEngine/MCJIT/remote/test-ptr-reloc-sm-pic.ll
LLVM :: ExecutionEngine/MCJIT/stubs-sm-pic.ll
LLVM :: ExecutionEngine/MCJIT/test-global-init-nonzero-sm-pic.ll
LLVM :: ExecutionEngine/MCJIT/test-ptr-reloc-sm-pic.ll
lhames edited edge metadata.Oct 31 2014, 2:40 PM

Hi Vladimir.

Sorry for the delayed response on this, and thanks for working on this.

I think this patch will need to be significantly re-worked before it can go in to the mainline. As it is currently written, it adds a lot of target specific fields and arguments to the RuntimeDyldELF class. We're hoping to reverse the trend of every architecture being lumped in to the one RuntimeDyldELF class. We want to implement future targets (and refactor existing targets) into target specific base classes. You can check out RuntimeDyldMachO and its subclasses for an idea of where we want to go. Some time in the next couple of weeks I'm hoping to break the X86 target out of RuntimeDyldELF and in to its own class (probably RuntimeDyldELFX86). When that happens, it might be useful for you to use as a template for a RuntimeDyldELFMips class.

A couple of specific questions: Why do you need the SymbolName field in RelocationEntry, and the IsLocal member in RelocationValueRef? We want to avoid bloating these classes, since they're shared by all targets. I may be able to suggest other places to attach/handle this information. If it really is required in those structs, I can try to plan my refactor so that RuntimeDyldMips can define its own versions.

Cheers,
Lang.

Hi Lang, thanks for the review.

Mips64 may have three relocations packed into one relocation record, where the result of one relocation is used as the addend for the next one, and they must be resolved in the order they appear.
The current code first resolves external symbols, so for mips64 this order would be broken.

E.g.:

000000000000000c R_MIPS_GPREL16 main
000000000000000c R_MIPS_SUB *ABS*
000000000000000c R_MIPS_HI16 *ABS*

The first relocation would be added to the Relocations, and the next two to the ExternalSymbolRelocations. Instead of using these two maps, we added a different container, one for all symbols (Mips64Relocations), with section id and RelocationEntry pairs. But then external symbols' name gets lost, and that's why we added a SymbolName field in the RelocationEntry.

For the IsLocal - a mips64 GOT entry for local symbols contains only high 16 bits of the symbol address, so we added this field to keep the track whether the symbol is local.

Regards,
Vladimir

dsanders edited edge metadata.Dec 22 2014, 9:29 AM

I recently decided to try a mips64 native build and found that there's only two kinds of 'make check-all' failures for this build at the moment. One of them is the lack of an MCJIT which this patch addresses. I've submitted a patch for the other.

I'll wait for a version of this patch that accounts for Lang's comments before reviewing in detail but there's a couple high-level comments I'd like to add.

lib/ExecutionEngine/RuntimeDyld/RuntimeDyldELF.cpp
1121–1129

We should take the ABI from the ELF e_flags (like PPC64 does) rather than from the triple. It should be possible to use all ABI's on all mips triples (e.g. N32 on a mips triple with -mcpu=mips64, or O32 on a mips64 triple). Admittedly this doesn't work right now but we're working on fixing it.

On a related note, this patch doesn't seem to distinguish N32 and N64. It looks like you're implementing N64 and silently treating N32 as if it were N64.

vstefanovic abandoned this revision.May 13 2015, 8:46 AM

Obsoleted by D9667.