[PATCH] D45633: [XRay] Typed event logging intrinsic

[Keith via Phabricator via llvm-commits]

kpw added a comment.

Adding some comments for questions I had. Please chime in if you can shed light on these.



================
Comment at: include/llvm/IR/Intrinsics.td:900
+def int_xray_typedevent : Intrinsic<[], [llvm_i16_ty, llvm_ptr_ty, llvm_i32_ty],
+                                        [NoCapture<1>, ReadOnly<1>, IntrWriteMem]>;
 //===----------------------------------------------------------------------===//
----------------
I confess I copied IntrWriteMem from the custom event intrinsic without understanding why it is the right choice.


================
Comment at: lib/Target/X86/X86MCInstLower.cpp:1098-1100
+      // FIXME: Add reordering to the stashing so we never clobber a register
+      // before we've stashed it. e.g. RDI may be the third argument of the
+      // caller.
----------------
The reason I didn't do this now is that I wasn't sure whether it should be done with a sled version change.

While it is obviously a change, it doesn't require a change in the patching/unpatching logic.


================
Comment at: test/CodeGen/X86/xray-typed-event-log.ll:13
+    %val = load i32, i32* %eventsize
+    call void @llvm.xray.typedevent(i16 %type, i8* %eventptr, i32 %val)
+    ; CHECK-LABEL: Lxray_typed_event_sled_0:
----------------
I would have liked to have an additional test for when the registers match up and the sled uses noops, but I couldn't figure out how to force that in IR.

I tried a variant with "call cc 78 void  ..." to try and force SystemV, but I think at the ir level, but it didn't have the desired effect. Any ideas?


Repository:
  rL LLVM

https://reviews.llvm.org/D45633