[PATCH] D36615: [XRay][CodeGen] Use PIC-friendly code in XRay sleds; remove synthetic references in .text

[Dean Michael Berris via Phabricator via llvm-commits]

dberris added inline comments.


================
Comment at: lib/Target/X86/X86MCInstLower.cpp:1100
+          EmitAndCountInstruction(
+              MCInstBuilder(X86::PUSH64r).addReg(UsedRegs[I]));
           EmitAndCountInstruction(MCInstBuilder(X86::MOV64rr)
----------------
pcc wrote:
> Here you are pushing rdi and rsi onto the stack whereas you weren't doing that before.
> 1) I don't understand why the pushes are necessary now when they weren't before.
> 2) Since you are pushing one more value than before, is the stack still aligned correctly?
Because this is an intrinsic being lowered, defined in `include/llvm/IR/Instrinsics.td`, we aren't quite able to provide a mask of the registers that will be clobbered/used by this routine. While we haven't quite run into bugs without stashing the `%rsi` and `%rdi` registers here (we do actually do the right thing in the trampoline), We also haven't been defining the calling convention for calls to this intrinsic (which we couldn't define for the intrinsic too). I think I'm just being a bit cautious here.

The alignment question is an interesting one. Because it's an intrinsic being lowered, and that we're already past the stack adjustment phase (and side-stepping the call-lowering code for normal call instructions), we haven't quite been tracking the stack adjustments here. What we do assume is that if the stack was aligned already, pushing two 64-bit values onto the stack will keep it aligned -- then the trampoline starts running, assuming that the stack is set up properly.

Do you have suggestions on how else to do/test this?


https://reviews.llvm.org/D36615