[PATCH] Patchpoint - support symbolic targets.

[Lang Hames]

Hi Philip,

Sorry about the delayed reply.

I'm going to commit as is. In the future though, I think we could chose the
code sequence based on the code-model. Small-code model would get a
PC-relative call, large would get the current code sequence.

Juergen - I'll update the docs before committing.

Thanks for the review guys!

Cheers,
Lang.


On Wed, Apr 8, 2015 at 3:27 PM, Philip Reames <listmail at philipreames.com>
wrote:

>  LGTM.  Since this is very similar to the statepoint code I used that for
> comparison sake.
>
> In our local code we've got extra handling for ExternalSymbolSDNode.  I'm
> not actually sure if that's required though.  :)
>
> Here's the line we have:
> else if (ExternalSymbolSDNode *ES =
> dyn_cast<ExternalSymbolSDNode>(CallTarget))
> {
>  CallTarget =
> DAG.getTargetExternalSymbol(
>
>     ES->getSymbol(), CallTarget.getValueType(),
> ES->getTargetFlags());
> }
>
> One slight tweak you could make is to use a pc relative call rather than a
> register call for symbolic targets.  This avoids the need for an extra
> register, but does require that the target be within a fixed offset.  (Or
> maybe this can be handled via relocations?  Not sure.)
>
> Philip
>
>
> On 03/31/2015 05:25 PM, Lang Hames wrote:
>
> Hi All,
>
>  At the moment llvm.patchpoint call targets must be integer constants
> (e.g. 0xDEADBEEF). This patch adds support for symbolic targets like @foo,
> which addresses a couple of FIXMEs.
>
>  Making this work just involved teaching FastISel and SelectionDAG to
> construct the appropriate MI/SDNodes, and teaching the Targets how to lower
> these to MCInsts. Target support for x86 is included in this patch. Support
> for other targets should be easy to derive from that.
>
>  With this patch applied, you can write patchpoints of the following form:
>
>  tail call i64 (i64, i32, i8*, i32, ...)*
>   @llvm.experimental.patchpoint.void(i64 9, i32 15,
>     i8* bitcast (i64 (i64, i64)* @foo to i8*),      ; <- Call target
>     i32 2, i64 %p1, i64 %p2)
>
>  and this will generate:
>
>  movabsq $_foo, %r11
> callq   *%r11
> // <nop-padding>
>
>  For integer targets this would have been something like:
>
>  movabsq $0xDEADBEEF, %r11
> callq   *%r11
> // <nop-padding>
>
>  The advantage of symbolic targets, beyond improved readability, is that
> you can cache the IR or compiled objects and re-use them in contexts where
> the target address may have changed. For example, objects that use symbolic
> patchpoints can be cached in Orc/MCJIT object-caches and re-used across JIT
> invocations.
>
>  Immediate-address targets are still fully supported.
>
>  I don't really see any downside to this patch, but I thought I'd throw
> it out for general discussion before I go and widen what patchpoint is
> supposed to support.
>
>  Does anyone see any problem with this idea?
>
>  For the curious, my motivation is that I'd like (eventually) to support
> patchpoints for JIT re-entry in Orc as an alternative to indirect calls,
> and I want that transformation to be straightforward. E.g.
>
>  call @not_yet_compiled, ...
> to
> call llvm.patchpoint ..., @not_yet_compiled, ...
>
>  Cheers,
> Lang.
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150421/46be06ba/attachment.html>