[PATCH] D27503: [XRay] Custom event logging intrinsic
Tim Shen via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Tue May 2 16:57:31 PDT 2017
timshen added a comment.
Other than some nits and questions, looks good. :)
================
Comment at: include/llvm/IR/Intrinsics.td:779
+// Takes a pointer to a string and the length of the string.
+def int_xray_customevent : Intrinsic<[], [llvm_ptr_ty, llvm_i8_ty],
+ [NoCapture<0>, ReadOnly<0>, IntrWriteMem]>;
----------------
Is the length going to be i8 forever? What if the strings are getting longer? Would it be better to use a word-size length variable?
================
Comment at: lib/CodeGen/SelectionDAG/FastISel.cpp:885
+bool FastISel::selectXRayCustomEvent(const CallInst *I) {
+ const auto &Triple = TM.getTargetTriple();
----------------
I thought It's not necessary to have a FastISel implementation for this, because if FastISel doesn't work, the execution falls back to SelectionDAG.
Now that you already have this, I'm not object to keeping it though. :)
================
Comment at: lib/Target/X86/X86MCInstLower.cpp:1052
+ // .p2align 1, ...
+ // .Lxray_event_sled_N:
+ // jmp +N // jump across the call instruction
----------------
I believe that before at symbol, all caller-saved registers are actually saved?
================
Comment at: lib/Target/X86/X86MCInstLower.cpp:1075
+ // FIXME: Find another less hacky way do force the relative jump.
+ OutStreamer->EmitBytes("\xeb\x14");
+
----------------
Maybe jump to an "exit" symbol, and emit that symbol at the end of the sled?
See: https://github.com/llvm-mirror/llvm/blob/288180ac40ab050711298f42340233bd47916634/lib/Target/PowerPC/PPCAsmPrinter.cpp#L1076
https://reviews.llvm.org/D27503
More information about the llvm-commits
mailing list