[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