[PATCH] Add VTune as an optional external dependency and add task tracking.

Michael Spencer bigcheesegs at gmail.com
Fri Apr 5 15:55:36 PDT 2013


  >I believe ScopedTask is being used to measure hotspots within those areas of code ?

  ScopedTask isn't for measuring hot spots. It's for tracking which time chunk of which thread is executing some logical task.

  >a) Why are ScopedTask present only in few places, arent they supposed to be placed everywhere in lld code ?

  I've only added them here where I happened to be testing at the time. They should only go in when you want to track a task, as they have a non-zero overhead with VTune. If you don't have VTune they are useless right now, but I plan to eventually implement simple timing stats based on them when you don't have VTune enabled (With a way to disable them so you have no overhead).


================
Comment at: cmake/modules/FindVTune.cmake:7-12
@@ +6,8 @@
+
+set(dirs
+  "$ENV{VTUNE_AMPLIFIER_XE_2013_DIR}/"
+  "C:/Program Files (x86)/Intel/VTune Amplifier XE 2013/"
+  "$ENV{VTUNE_AMPLIFIER_XE_2011_DIR}/"
+  "C:/Program Files (x86)/Intel/VTune Amplifier XE 2011/"
+  )
+
----------------
Shankar Kalpathi Easwaran wrote:
> Should this be under MSVC, because the paths are ms-windows paths ?
They don't need to be conditional. Other systems will just ignore them.

================
Comment at: include/lld/Core/Instrumentation.h:15-16
@@ +14,4 @@
+
+#ifndef LLD_CORE_TASK_TRACKING_H
+#define LLD_CORE_TASK_TRACKING_H
+
----------------
Shankar Kalpathi Easwaran wrote:
> Should this be named LLD_CORE_INSTRUMENTATION_H ?
Yes

================
Comment at: include/lld/Core/Instrumentation.h:38-61
@@ +37,26 @@
+
+class StringHandle {
+  __itt_string_handle *_handle;
+
+public:
+  StringHandle(const char *name) : _handle(__itt_string_handle_createA(name)) {}
+
+  operator __itt_string_handle *() const { return _handle; }
+};
+
+class ScopedTask {
+  __itt_domain *_domain;
+
+public:
+  ScopedTask(const Domain &d, const StringHandle &s) : _domain(d) {
+    __itt_task_begin(d, __itt_null, __itt_null, s);
+  }
+
+  void end() {
+    if (_domain)
+      __itt_task_end(_domain);
+    _domain = nullptr;
+  }
+
+  ~ScopedTask() { end(); }
+};
----------------
Shankar Kalpathi Easwaran wrote:
> Can you document these classes and their purposes ?
k

================
Comment at: lib/Passes/LayoutPass.cpp:16-17
@@ -15,2 +15,4 @@
 
+#include "lld/Core/Instrumentation.h"
+
 #include "llvm/Support/Debug.h"
----------------
Shankar Kalpathi Easwaran wrote:
> not in sorted order, should be before line 13.
This is LayoutPass.cpp. The main header for the cpp always goes first. Although The duplicate Debug.h needs to go.


http://llvm-reviews.chandlerc.com/D634



More information about the llvm-commits mailing list