<p dir="ltr">I am OK with this if it is cumbersome to use a non ref counted timer in the current users.</p>
<div class="gmail_quote">On May 27, 2016 1:11 PM, "Vedant Kumar" <<a href="mailto:vsk@apple.com">vsk@apple.com</a>> wrote:<br type="attribution"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">vsk added a comment.<br>
<br>
@rafael we don't know how hard it would be to change users which expect to be able to nest {start,stop}Timer. In CodeGenAction.cpp alone it seems like there could be many such users.<br>
<br>
> The real worry on my side is that I'm not aware of all the possible cases where we rely on this behavior.<br>
<br>
<br>
Right, that's what I'm concerned about too. I don't see any advantages of keeping the stricter (i.e non-nestable) {start,stop}Timer implementation.<br>
<br>
<br>
================<br>
Comment at: include/llvm/Support/Timer.h:122<br>
@@ +121,3 @@<br>
+  TimeRecord getTotalTime() const {<br>
+    assert(RefCount == 0 && "Cannot query active timer");<br>
+    return Time;<br>
----------------<br>
davide wrote:<br>
> Any reason why you can't query an active timer?<br>
You'd get back a negative time, since startTimer() does `Time -= CurrentTime`.<br>
<br>
================<br>
Comment at: lib/Support/Timer.cpp:155<br>
@@ -151,3 +154,3 @@<br>
 void Timer::clear() {<br>
-  Running = Triggered = false;<br>
-  Time = StartTime = TimeRecord();<br>
+  RefCount = 0;<br>
+  Triggered = false;<br>
----------------<br>
davide wrote:<br>
> hmm, imagine the following sequence:<br>
> startTimer()<br>
> startTimer()<br>
> clear()<br>
> stop()<br>
> stop()<br>
><br>
> You hit an assertion, no?<br>
Yes. It seems correct to me that you'd hit an assertion in that case. clear() would reset the total time, so there is nothing to stop.<br>
<br>
<br>
<a href="http://reviews.llvm.org/D20735" rel="noreferrer" target="_blank">http://reviews.llvm.org/D20735</a><br>
<br>
<br>
<br>
</blockquote></div>