Hi Andrew, I wrote the draft of this e-mail few weeks ago, and the following sentence is not true: > > Its emitted when the new MIN falls inside the (MIN..MAX) range of the OLD > baseline. It is not checked the other way around. > See below...
On Tue, Jun 13, 2017 at 1:45 AM, Pavol Vaskovic <p...@pali.sk> wrote: > Hi Andrew, > > On Mon, Jun 12, 2017 at 11:55 PM, Andrew Trick <atr...@apple.com> wrote: > >> To partially address this issue (I'm guessing) the last SPEEDUP column >> sometimes features mysterious question mark in brackets. Its emitted when >> the new MIN falls inside the (MIN..MAX) range of the OLD baseline. It is >> not checked the other way around. >> >> >> That bug must have been introduced during one of the rewrites. Is that in >> the driver or compare script? Why not fix that bug? >> > > That is in the compare script. It looks like the else branch got lost during > a rewrite > <https://github.com/apple/swift/commit/cb23837bb932f21b61d2a79c936d88c167fd91d0#diff-5ca4ab28608a4259eff23c72eed7ae8d> > (search > for "(?)" in that diff). I could certainly fix that too, but I'm not sure > that would be enough to fix all our problems. > I even wrote tests for this <https://github.com/apple/swift/blob/686c7619922d0de2bfdb3a0ec988b2c3c6bb60b5/benchmark/scripts/test_compare_perf_tests.py#L162>, and my implementation is pretty clear too <https://github.com/apple/swift/blob/e7b243cad7e27af63e6c25cd426fdc5359c4e51d/benchmark/scripts/compare_perf_tests.py#L135>… somehow I forgot this. # Add ' (?)' to the speedup column as indication of dubious changes: > > # result's MIN falls inside the (MIN, MAX) interval of result they are > > # being compared with. > > self.is_dubious = ( > > ' (?)' if ((old.min < new.min and new.min < old.max) or > > (new.min < old.min and old.min < new.max)) > > else '') > > I'm sorry for the confusion. --Pavol
_______________________________________________ swift-dev mailing list swift-dev@swift.org https://lists.swift.org/mailman/listinfo/swift-dev
