[llvm] r235483 - [patchpoint] Add support for symbolic patchpoint targets to SelectionDAG and the
Lang Hames
lhames at gmail.com
Thu Jul 16 12:42:55 PDT 2015
Oops - failed to hit send. Sorry about the delayed reply. ><
Hi Hal,
> Wouldn't it be better just to emit:
>
> call _foo
Sounds good to me.
Quick background: When I originally wrote this I decided to match the
non-symbolic code sequence so that I could be 100% certain not to break
anyone. Reading http://llvm.org/docs/StackMaps.html though, I see that
clients have never been allowed to assume anything about the code sequence,
so this should be safe.
Cheers,
Lang.
On Sat, Jul 11, 2015 at 9:18 PM, Hal Finkel <hfinkel at anl.gov> wrote:
> ----- Original Message -----
> > From: "Lang Hames" <lhames at gmail.com>
> > To: llvm-commits at cs.uiuc.edu
> > Sent: Wednesday, April 22, 2015 1:02:32 AM
> > Subject: [llvm] r235483 - [patchpoint] Add support for symbolic
> patchpoint targets to SelectionDAG and the
> >
> > Author: lhames
> > Date: Wed Apr 22 01:02:31 2015
> > New Revision: 235483
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=235483&view=rev
> > Log:
> > [patchpoint] Add support for symbolic patchpoint targets to
> > SelectionDAG and the
> > X86 backend.
> >
> > The code generated for symbolic targets is identical to the code
> > generated for
> > constant targets, except that a relocation is emitted to fix up the
> > actual
> > target address at link-time. This allows IR and object files
> > containing
> > patchpoints to be cached across JIT-invocations where the target
> > address may
> > change.
> >
> >
> > Modified:
> > llvm/trunk/include/llvm/MC/MCInstBuilder.h
> > llvm/trunk/lib/CodeGen/SelectionDAG/FastISel.cpp
> > llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
> > llvm/trunk/lib/Target/X86/X86AsmPrinter.h
> > llvm/trunk/lib/Target/X86/X86MCInstLower.cpp
> > llvm/trunk/test/CodeGen/X86/patchpoint.ll
> >
> > Modified: llvm/trunk/include/llvm/MC/MCInstBuilder.h
> > URL:
> >
> http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/MC/MCInstBuilder.h?rev=235483&r1=235482&r2=235483&view=diff
> >
> ==============================================================================
> > --- llvm/trunk/include/llvm/MC/MCInstBuilder.h (original)
> > +++ llvm/trunk/include/llvm/MC/MCInstBuilder.h Wed Apr 22 01:02:31
> > 2015
> > @@ -58,6 +58,12 @@ public:
> > return *this;
> > }
> >
> > + /// \brief Add an operand.
> > + MCInstBuilder &addOperand(const MCOperand &Op) {
> > + Inst.addOperand(Op);
> > + return *this;
> > + }
> > +
> > operator MCInst&() {
> > return Inst;
> > }
> >
> > Modified: llvm/trunk/lib/CodeGen/SelectionDAG/FastISel.cpp
> > URL:
> >
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/SelectionDAG/FastISel.cpp?rev=235483&r1=235482&r2=235483&view=diff
> >
> ==============================================================================
> > --- llvm/trunk/lib/CodeGen/SelectionDAG/FastISel.cpp (original)
> > +++ llvm/trunk/lib/CodeGen/SelectionDAG/FastISel.cpp Wed Apr 22
> > 01:02:31 2015
> > @@ -711,7 +711,7 @@ bool FastISel::selectPatchpoint(const Ca
> > CallingConv::ID CC = I->getCallingConv();
> > bool IsAnyRegCC = CC == CallingConv::AnyReg;
> > bool HasDef = !I->getType()->isVoidTy();
> > - Value *Callee = I->getOperand(PatchPointOpers::TargetPos);
> > + Value *Callee =
> > I->getOperand(PatchPointOpers::TargetPos)->stripPointerCasts();
> >
> > // Get the real number of arguments participating in the call
> > <numArgs>
> > assert(isa<ConstantInt>(I->getOperand(PatchPointOpers::NArgPos))
> > &&
> > @@ -757,23 +757,25 @@ bool FastISel::selectPatchpoint(const Ca
> > cast<ConstantInt>(I->getOperand(PatchPointOpers::NBytesPos));
> > Ops.push_back(MachineOperand::CreateImm(NumBytes->getZExtValue()));
> >
> > - // Assume that the callee is a constant address or null pointer.
> > - // FIXME: handle function symbols in the future.
> > - uint64_t CalleeAddr;
> > - if (const auto *C = dyn_cast<IntToPtrInst>(Callee))
> > - CalleeAddr =
> > cast<ConstantInt>(C->getOperand(0))->getZExtValue();
> > - else if (const auto *C = dyn_cast<ConstantExpr>(Callee)) {
> > - if (C->getOpcode() == Instruction::IntToPtr)
> > - CalleeAddr =
> > cast<ConstantInt>(C->getOperand(0))->getZExtValue();
> > - else
> > + // Add the call target.
> > + if (const auto *C = dyn_cast<IntToPtrInst>(Callee)) {
> > + uint64_t CalleeConstAddr =
> > + cast<ConstantInt>(C->getOperand(0))->getZExtValue();
> > + Ops.push_back(MachineOperand::CreateImm(CalleeConstAddr));
> > + } else if (const auto *C = dyn_cast<ConstantExpr>(Callee)) {
> > + if (C->getOpcode() == Instruction::IntToPtr) {
> > + uint64_t CalleeConstAddr =
> > + cast<ConstantInt>(C->getOperand(0))->getZExtValue();
> > + Ops.push_back(MachineOperand::CreateImm(CalleeConstAddr));
> > + } else
> > llvm_unreachable("Unsupported ConstantExpr.");
> > + } else if (const auto *GV = dyn_cast<GlobalValue>(Callee)) {
> > + Ops.push_back(MachineOperand::CreateGA(GV, 0));
> > } else if (isa<ConstantPointerNull>(Callee))
> > - CalleeAddr = 0;
> > + Ops.push_back(MachineOperand::CreateImm(0));
> > else
> > llvm_unreachable("Unsupported callee address.");
> >
> > - Ops.push_back(MachineOperand::CreateImm(CalleeAddr));
> > -
> > // Adjust <numArgs> to account for any arguments that have been
> > passed on
> > // the stack instead.
> > unsigned NumCallRegArgs = IsAnyRegCC ? NumArgs :
> > CLI.OutRegs.size();
> >
> > Modified: llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
> > URL:
> >
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp?rev=235483&r1=235482&r2=235483&view=diff
> >
> ==============================================================================
> > --- llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
> > (original)
> > +++ llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp Wed
> > Apr 22 01:02:31 2015
> > @@ -7001,7 +7001,16 @@ void SelectionDAGBuilder::visitPatchpoin
> > CallingConv::ID CC = CS.getCallingConv();
> > bool IsAnyRegCC = CC == CallingConv::AnyReg;
> > bool HasDef = !CS->getType()->isVoidTy();
> > - SDValue Callee = getValue(CS->getOperand(2)); // <target>
> > + SDValue Callee =
> > getValue(CS->getOperand(PatchPointOpers::TargetPos));
> > +
> > + // Handle immediate and symbolic callees.
> > + if (auto* ConstCallee = dyn_cast<ConstantSDNode>(Callee))
> > + Callee = DAG.getIntPtrConstant(ConstCallee->getZExtValue(),
> > + /*isTarget=*/true);
> > + else if (auto* SymbolicCallee =
> > dyn_cast<GlobalAddressSDNode>(Callee))
> > + Callee =
> > DAG.getTargetGlobalAddress(SymbolicCallee->getGlobal(),
> > + SDLoc(SymbolicCallee),
> > +
> > SymbolicCallee->getValueType(0));
> >
> > // Get the real number of arguments participating in the call
> > <numArgs>
> > SDValue NArgVal =
> > getValue(CS.getArgument(PatchPointOpers::NArgPos));
> > @@ -7041,11 +7050,8 @@ void SelectionDAGBuilder::visitPatchpoin
> > Ops.push_back(DAG.getTargetConstant(
> > cast<ConstantSDNode>(NBytesVal)->getZExtValue(),
> > MVT::i32));
> >
> > - // Assume that the Callee is a constant address.
> > - // FIXME: handle function symbols in the future.
> > - Ops.push_back(
> > -
> > DAG.getIntPtrConstant(cast<ConstantSDNode>(Callee)->getZExtValue(),
> > - /*isTarget=*/true));
> > + // Add the callee.
> > + Ops.push_back(Callee);
> >
> > // Adjust <numArgs> to account for any arguments that have been
> > passed on the
> > // stack instead.
> >
> > Modified: llvm/trunk/lib/Target/X86/X86AsmPrinter.h
> > URL:
> >
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/X86AsmPrinter.h?rev=235483&r1=235482&r2=235483&view=diff
> >
> ==============================================================================
> > --- llvm/trunk/lib/Target/X86/X86AsmPrinter.h (original)
> > +++ llvm/trunk/lib/Target/X86/X86AsmPrinter.h Wed Apr 22 01:02:31
> > 2015
> > @@ -81,7 +81,7 @@ class LLVM_LIBRARY_VISIBILITY X86AsmPrin
> >
> > void InsertStackMapShadows(MachineFunction &MF);
> > void LowerSTACKMAP(const MachineInstr &MI);
> > - void LowerPATCHPOINT(const MachineInstr &MI);
> > + void LowerPATCHPOINT(const MachineInstr &MI, X86MCInstLower
> > &MCIL);
> >
> > void LowerTlsAddr(X86MCInstLower &MCInstLowering, const
> > MachineInstr &MI);
> >
> >
> > Modified: llvm/trunk/lib/Target/X86/X86MCInstLower.cpp
> > URL:
> >
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/X86MCInstLower.cpp?rev=235483&r1=235482&r2=235483&view=diff
> >
> ==============================================================================
> > --- llvm/trunk/lib/Target/X86/X86MCInstLower.cpp (original)
> > +++ llvm/trunk/lib/Target/X86/X86MCInstLower.cpp Wed Apr 22 01:02:31
> > 2015
> > @@ -867,7 +867,8 @@ void X86AsmPrinter::LowerSTACKMAP(const
> >
> > // Lower a patchpoint of the form:
> > // [<def>], <id>, <numBytes>, <target>, <numArgs>, <cc>, ...
> > -void X86AsmPrinter::LowerPATCHPOINT(const MachineInstr &MI) {
> > +void X86AsmPrinter::LowerPATCHPOINT(const MachineInstr &MI,
> > + X86MCInstLower &MCIL) {
> > assert(Subtarget->is64Bit() && "Patchpoint currently only supports
> > X86-64");
> >
> > SMShadowTracker.emitShadowPadding(OutStreamer,
> > getSubtargetInfo());
> > @@ -877,8 +878,29 @@ void X86AsmPrinter::LowerPATCHPOINT(cons
> > PatchPointOpers opers(&MI);
> > unsigned ScratchIdx = opers.getNextScratchIdx();
> > unsigned EncodedBytes = 0;
> > - int64_t CallTarget =
> > opers.getMetaOper(PatchPointOpers::TargetPos).getImm();
> > - if (CallTarget) {
> > + const MachineOperand &CalleeMO =
> > + opers.getMetaOper(PatchPointOpers::TargetPos);
> > +
> > + // Check for null target. If target is non-null (i.e. is non-zero
> > or is
> > + // symbolic) then emit a call.
> > + if (!(CalleeMO.isImm() && !CalleeMO.getImm())) {
> > + MCOperand CalleeMCOp;
> > + switch (CalleeMO.getType()) {
> > + default:
> > + /// FIXME: Add a verifier check for bad callee types.
> > + llvm_unreachable("Unrecognized callee operand type.");
> > + case MachineOperand::MO_Immediate:
> > + if (CalleeMO.getImm())
> > + CalleeMCOp = MCOperand::CreateImm(CalleeMO.getImm());
> > + break;
> > + case MachineOperand::MO_ExternalSymbol:
> > + case MachineOperand::MO_GlobalAddress:
> > + CalleeMCOp =
> > + MCIL.LowerSymbolOperand(CalleeMO,
> > +
> > MCIL.GetSymbolFromOperand(CalleeMO));
> > + break;
> > + }
> > +
> > // Emit MOV to materialize the target address and the CALL to
> > target.
> > // This is encoded with 12-13 bytes, depending on which register
> > is used.
> > unsigned ScratchReg = MI.getOperand(ScratchIdx).getReg();
> > @@ -886,10 +908,12 @@ void X86AsmPrinter::LowerPATCHPOINT(cons
> > EncodedBytes = 13;
> > else
> > EncodedBytes = 12;
> > -
> > EmitAndCountInstruction(MCInstBuilder(X86::MOV64ri).addReg(ScratchReg)
> > -
> >
> .addImm(CallTarget));
> > +
> > + EmitAndCountInstruction(
> > +
> >
> MCInstBuilder(X86::MOV64ri).addReg(ScratchReg).addOperand(CalleeMCOp));
> >
> EmitAndCountInstruction(MCInstBuilder(X86::CALL64r).addReg(ScratchReg));
> > }
> > +
> > // Emit padding.
> > unsigned NumBytes =
> > opers.getMetaOper(PatchPointOpers::NBytesPos).getImm();
> > assert(NumBytes >= EncodedBytes &&
> > @@ -1091,7 +1115,7 @@ void X86AsmPrinter::EmitInstruction(cons
> > return LowerSTACKMAP(*MI);
> >
> > case TargetOpcode::PATCHPOINT:
> > - return LowerPATCHPOINT(*MI);
> > + return LowerPATCHPOINT(*MI, MCInstLowering);
> >
> > case X86::MORESTACK_RET:
> > EmitAndCountInstruction(MCInstBuilder(getRetOpcode(*Subtarget)));
> >
> > Modified: llvm/trunk/test/CodeGen/X86/patchpoint.ll
> > URL:
> >
> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/patchpoint.ll?rev=235483&r1=235482&r2=235483&view=diff
> >
> ==============================================================================
> > --- llvm/trunk/test/CodeGen/X86/patchpoint.ll (original)
> > +++ llvm/trunk/test/CodeGen/X86/patchpoint.ll Wed Apr 22 01:02:31
> > 2015
> > @@ -21,6 +21,22 @@ entry:
> > ret i64 %result
> > }
> >
> > +; Trivial symbolic patchpoint codegen.
> > +;
> > +
> > +declare i64 @foo(i64 %p1, i64 %p2)
> > +define i64 @trivial_symbolic_patchpoint_codegen(i64 %p1, i64 %p2) {
> > +entry:
> > +; CHECK-LABEL: trivial_symbolic_patchpoint_codegen:
> > +; CHECK: movabsq $_foo, %r11
> > +; CHECK-NEXT: callq *%r11
>
> Wouldn't it be better just to emit:
>
> callq _foo
>
> instead of the move and then the indirect call?
>
> -Hal
>
> > +; CHECK-NEXT: xchgw %ax, %ax
> > +; CHECK: retq
> > + %result = tail call i64 (i64, i32, i8*, i32, ...)
> > @llvm.experimental.patchpoint.i64(i64 9, i32 15, i8* bitcast (i64
> > (i64, i64)* @foo to i8*), i32 2, i64 %p1, i64 %p2)
> > + ret i64 %result
> > +}
> > +
> > +
> > ; Caller frame metadata with stackmaps. This should not be optimized
> > ; as a leaf function.
> > ;
> >
> >
> > _______________________________________________
> > llvm-commits mailing list
> > llvm-commits at cs.uiuc.edu
> > http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
> >
>
> --
> Hal Finkel
> Assistant Computational Scientist
> Leadership Computing Facility
> Argonne National Laboratory
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150716/c321aa68/attachment.html>
More information about the llvm-commits
mailing list