[llvm-commits] [RFC] PPC64 TOC and MCJIT support
Will Schmidt
will_schmidt at vnet.ibm.com
Wed Aug 8 11:31:02 PDT 2012
On Mon, 2012-08-06 at 15:11 -0300, Adhemerval Zanella wrote:
> Although the patch is still WIP and requires a lot of work, I'd appreciate some
> advices and feedback for my modifications. As I stated em previous emails, although
> llvm generates working code for PPC32, it lacks some functionality for PPC64,
> mainly the TOC usage for PIC and PIE generation.
>
> This patch is based on previous email and feedback I got to add support for TOC
> and the PPC64 relocation needed for both JIT and assembly. Currently the assembly
> generated creates the @toc relocation correctly instead of @ha/@lo, that works
> on mostly cases, but are not full PIC and PIE.
>
> The testcase result remains the same, so next step I add PPC64 toc testcase
> to fully test the TOC code generation.
Thanks for posting. A few mostly cosmetic comments below.
> ---
>
> Index: include/llvm/Object/ELF.h
> ===================================================================
> Index: include/llvm/Support/ELF.h
> ===================================================================
> Index: lib/Target/PowerPC/PPCCodeEmitter.cpp
> ===================================================================
> Index: lib/Target/PowerPC/MCTargetDesc/PPCMCCodeEmitter.cpp
> ===================================================================
> Index: lib/Target/PowerPC/MCTargetDesc/PPCAsmBackend.cpp
> ===================================================================
> --- lib/Target/PowerPC/MCTargetDesc/PPCAsmBackend.cpp (revision 161325)
> +++ lib/Target/PowerPC/MCTargetDesc/PPCAsmBackend.cpp (working copy)
> @@ -29,7 +29,11 @@
> case FK_Data_1:
> case FK_Data_2:
> case FK_Data_4:
> + case FK_Data_8:
> return Value;
> + case PPC::fixup_ppc_lo14:
> + case PPC::fixup_ppc_toc_lo14:
> + return (Value & 0xffff) >> 2;
Is it possible to, and would it make sense to replace with
ppc_lo(Value) >> 2 ? (That you have defined in RuntimeDyldELF.cpp )
> case PPC::fixup_ppc_brcond14:
> return Value & 0x3ffc;
> case PPC::fixup_ppc_br24:
> @@ -41,6 +45,7 @@
> case PPC::fixup_ppc_ha16:
> return ((Value >> 16) + ((Value & 0x8000) ? 1 : 0)) & 0xffff;
And similar question here on swapping in ppc_ha(Value) for (Value &
0x8000 ).
> case PPC::fixup_ppc_lo16:
> + case PPC::fixup_ppc_toc_lo16:
> return Value & 0xffff;
> Index: lib/Target/PowerPC/MCTargetDesc/PPCFixupKinds.h
> ===================================================================
> Index: lib/Target/PowerPC/MCTargetDesc/PPCELFObjectWriter.cpp
> ===================================================================
> Index: lib/Target/PowerPC/PPCRelocations.h
> ===================================================================
> Index: lib/Target/PowerPC/PPCInstr64Bit.td
> ===================================================================
> Index: lib/Target/PowerPC/PPCISelLowering.cpp
> ===================================================================
> --- lib/Target/PowerPC/PPCISelLowering.cpp (revision 161325)
> +++ lib/Target/PowerPC/PPCISelLowering.cpp (working copy)
> @@ -106,7 +106,7 @@
> // from FP_ROUND: that rounds to nearest, this rounds to zero.
> setOperationAction(ISD::FP_ROUND_INREG, MVT::ppcf128, Custom);
>
> - // We do not currently implment this libm ops for PowerPC.
> + // We do not currently implement this libm ops for PowerPC.
s/this/these/ ?
> setOperationAction(ISD::FFLOOR, MVT::ppcf128, Expand);
> setOperationAction(ISD::FCEIL, MVT::ppcf128, Expand);
> setOperationAction(ISD::FTRUNC, MVT::ppcf128, Expand);
> @@ -1204,6 +1204,14 @@
> ConstantPoolSDNode *CP = cast<ConstantPoolSDNode>(Op);
> const Constant *C = CP->getConstVal();
>
> + // 64-bit SVR4 ABI code is always position-independent.
> + // The actual address of the GlobalValue is stored in the TOC.
> + if (PPCSubTarget.isSVR4ABI() && PPCSubTarget.isPPC64()) {
> + SDValue GA = DAG.getTargetConstantPool(C, PtrVT, CP->getAlignment(), 0);
> + return DAG.getNode(PPCISD::TOC_ENTRY, CP->getDebugLoc(), MVT::i64, GA,
> + DAG.getRegister(PPC::X2, MVT::i64));
> + }
^ this code chunk is duplicated below. Worthy of a macro or being
broken out into its own function?
> +
> unsigned MOHiFlag, MOLoFlag;
> bool isPIC = GetLabelAccessInfo(DAG.getTarget(), MOHiFlag, MOLoFlag);
> SDValue CPIHi =
> @@ -1217,6 +1225,14 @@
> EVT PtrVT = Op.getValueType();
> JumpTableSDNode *JT = cast<JumpTableSDNode>(Op);
>
> + // 64-bit SVR4 ABI code is always position-independent.
> + // The actual address of the GlobalValue is stored in the TOC.
> + if (PPCSubTarget.isSVR4ABI() && PPCSubTarget.isPPC64()) {
> + SDValue GA = DAG.getTargetJumpTable(JT->getIndex(), PtrVT);
> + return DAG.getNode(PPCISD::TOC_ENTRY, JT->getDebugLoc(), MVT::i64, GA,
> + DAG.getRegister(PPC::X2, MVT::i64));
^ duplicate is here
> + }
> +
> unsigned MOHiFlag, MOLoFlag;
> bool isPIC = GetLabelAccessInfo(DAG.getTarget(), MOHiFlag, MOLoFlag);
> SDValue JTIHi = DAG.getTargetJumpTable(JT->getIndex(), PtrVT, MOHiFlag);
> Index: lib/Target/PowerPC/PPCAsmPrinter.cpp
> ===================================================================
> Index: lib/ExecutionEngine/RuntimeDyld/RuntimeDyldELF.cpp
> ===================================================================
> +void RuntimeDyldELF::resolvePPC64Relocation(uint8_t *LocalAddress,
> + uint64_t FinalAddress,
> + uint64_t Value,
> + uint32_t Type,
> + int64_t Addend) {
> + switch (Type) {
> + default:
> + llvm_unreachable("Relocation type not implemented yet!");
> + break;
> + case ELF::R_PPC64_ADDR16_LO :
> + *(uint16_t*)(LocalAddress) = ppc_lo (Value);
> + break;
> + case ELF::R_PPC64_ADDR16_HA :
> + *(uint16_t*)(LocalAddress) = ppc_ha (Value);
> + break;
> + case ELF::R_PPC64_ADDR14 :
> + {
> + if (((Value + 0x8000) >= 0x10000) || ((Value & 3) != 0))
> + llvm_unreachable("Relocation R_PPC64_ADDR14 overflow");
> + int64_t insn = *(int64_t*)(LocalAddress);
> + // Performs (insn & FFFFFFFFFFFF0003) | (Value & 0xFFFC)
> + // On branch, bit 62 and 63 are ignoned
s/ignoned/ignored/
> + insn = ppc_bit_insert (insn, Value, 0xFFFC);
> + insn &= ~(1 << 21);
> + // insn & (0x14 << 21) checks is 'a' bit in branch is set
> + // 0 means no hint is given
> + if ((insn & (0x14 << 21)) == (0x04 << 21))
> + // Sets the branch is very likely to be taken
> + insn |= 0x02 << 21;
> + else if ((insn & (0x14 << 21)) == (0x10 << 21))
> + // Sets the branch is very likely to not be taken
> + insn |= 0x08 << 21;
> + *(int64_t*)(LocalAddress) = insn;
> + } break;
Fully realizing that you added the comments here by my request, (thanks!
), I think commenting each line may be overkill. i.e.
+ int64_t insn = *(int64_t*)(LocalAddress);
> + // On branch, bit 62 and 63 are ignored
> + insn = ppc_bit_insert (insn, Value, 0xFFFC);
> + insn &= ~(1 << 21);
> + // Check for a branch prediction hint via the 'a' bit in the branch instruction.
> + // A 0 means no hint is given, a 0x04 indicates we predict
// the branch to be taken, a 0x10 indicates we predict not
// to take the branch.
> + if ((insn & (0x14 << 21)) == (0x04 << 21))
> + insn |= 0x02 << 21;
> + else if ((insn & (0x14 << 21)) == (0x10 << 21))
> Index: lib/ExecutionEngine/RuntimeDyld/RuntimeDyldELF.h
> ===================================================================
Thanks,
-Will
More information about the llvm-commits
mailing list