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

[Dean Michael Berris via llvm-commits]

dberris added inline comments.

================
Comment at: include/llvm/Target/TargetInstrInfo.h:155-166
@@ -154,1 +154,14 @@
 
+  /// This returns true if the given instruction is a "normal return" as opposed
+  /// to special kinds of returns. On platforms where there may be multiple
+  /// OpCodes that signify 'normal' return instructions (like in X86), this
+  /// return true for those instructions. The default implementation excludes
+  /// known non-normal instructions.
+  virtual bool isNormalReturn(const MachineInstr *MI) const {
+    if (!MI->isReturn()) return false;
+    auto OpCode = MI->getOpcode();
+    bool IsCatchReturn = OpCode == getCatchReturnOpcode();
+    bool IsFrameDestroy = OpCode == getCallFrameDestroyOpcode();
+    return !IsCatchReturn && !IsFrameDestroy;
+  }
+
----------------
majnemer wrote:
> I was thinking more of a whilelist-oriented solution, something like:
>   `unsigned getNormalReturnOpcode() const { return NormalRetOpcode; }`
But how does this work on platforms that can spell 'return' many different normal ways (like in X86)? There's RETL, RETQ, and all other versions of RET which are considered "normal"?

================
Comment at: lib/Target/X86/X86MCInstLower.cpp:1082
@@ +1081,3 @@
+  //   jmp .tmpN
+  //   # 9 bytes worth of noops
+  // .tmpN
----------------
bmakam wrote:
> Could you please expand on why you need 9 bytes of noops here? I am not quite familiar with x86_64 but was under the impression that on x86_64 the jmp instruction is 1 byte for opcode and 4 bytes for signed relative displacement, so shouldn't 5 bytes worth of nops be sufficient?
Good question, thanks.

I have to check whether we're using the right version of JMP, but I'm specifically looking for the version that's one byte for the JMP instruction, and 8 bits (1 byte) for the relative offset. So far I haven't been able to spell `jmp +0x09` correctly and have it work, without having an additional symbol as a target for the jump instruction. If we get that right, we can then add the 9 byte nops we need to get exactly 11 bytes for the function entry.

Is there a fool-proof way of spelling "JMP +0x09" with the builder interface? Or should I add another JMP instruction in X86 that supports the 8-bit displacement immediate operand?

The reason why I can't use a JMP that isn't 2 bytes, is because it's really hard to write just 5 bytes atomically. I can probably do something with an 8-byte atomic write, but enforcing that 8-byte write doesn't span cache lines is also very tricky to make safe.


http://reviews.llvm.org/D19904