[PATCH] D19904: XRay: Add entry and exit sleds

[Dean Michael Berris via llvm-commits]

dberris marked 2 inline comments as done.
dberris added a comment.

Thanks Reid -- do you or echristo@ mind landing this for me? I don't think I have commit powers yet.


================
Comment at: lib/Target/X86/X86MCInstLower.cpp:1051
@@ +1050,3 @@
+  //   mov %r10, <function id, 32-bit>   // 6 bytes
+  //   call <relative offset, 32-bits>   // 5 bytes
+  //
----------------
rnk wrote:
> Does XRay support multiple DSOs in the process? If so, the XRay runtime may be more than 2GB away from the code being patched, and this offset will overflow.
That's true -- the runtime patching code will fail to patch certain sleds that are farther than 32-bits away. In case we need more, in the future we can tweak this so that we can try to force the XRay runtime trampoline in the first 2GB of the process' memory and use an "absolute" address (which may need more bytes in the sled), or have a mode to support larger binaries (a flag that changes the emitted sleds, and have the runtime support both kinds of sleds).

I suspect if it comes up in practice that we can deal with this more effectively.

================
Comment at: lib/Target/X86/X86MCInstLower.cpp:1060-1061
@@ +1059,4 @@
+  // an operand (computed as an offset from the jmp instruction).
+  // FIXME: Find another less hacky way do force the relative jump.
+  OutStreamer->EmitBytes("\xeb\x09");
+  EmitNops(*OutStreamer, 9, Subtarget->is64Bit(), getSubtargetInfo());
----------------
rnk wrote:
> I guess we can deal with this separately. IMO we should have a 'jmpb' instruction or something that forces a short jump or assembler error if the displacement is too large.
I'm happy to do this as a refactoring or a follow-up change. A new instruction sounds like the right action to take anyway.


http://reviews.llvm.org/D19904