This is an archive of the discontinued LLVM Phabricator instance.

[libc++] Fix typo in std::midpoint
ClosedPublic

Authored by ruslo on Dec 15 2019, 8:27 AM.

Details

Summary

Fix typo in std::midpoint

Diff Detail

Event Timeline

ruslo created this revision.Dec 15 2019, 8:27 AM
Herald added a project: Restricted Project. · View Herald Transcript

Not covered by tests ? :/

ruslo added a comment.EditedDec 15 2019, 8:59 AM

I haven't succeeded in creating a test case that will cover this error. Probably in practice, there is no visible error because the difference is between (a/2 + b/2) and (a/2 + b) when (a > hi && b < lo). In both cases, it will be equal to a/2 because b is too small to contribute anything.

there is no visible error because the difference is between (a/2 + b/2) and (a/2 + b) when (a > hi && b < lo)

You can see the difference if you run the benchmark:

#include <iostream>
#include <numeric>
#include <vector>

using namespace std;

double midpoint_sum(const vector<double>& v1, const vector<double>& v2) {
  double res{0.0};
  for (size_t i = 0; i < v1.size(); ++i) {
    res += midpoint(v1[i], v2[i]);
  }
  return res;
}

template <class T>
void print_duration(T start, T end) {
  auto diff = end - start;
  cout << chrono::duration_cast<chrono::milliseconds>(diff).count() << " ms\n";
}

int main() {
  using clock = chrono::high_resolution_clock;

  size_t n = 500 * 1000 * 1000;

  double a = numeric_limits<double>::min() * 1.99;  // a < lo
  double b = numeric_limits<double>::max() / 1.99;  // b > hi

  vector<double> v_a(n, a);
  vector<double> v_b(n, b);

  auto start = clock::now();
  double r_a = midpoint_sum(v_a, v_b);
  auto end = clock::now();
  print_duration(start, end);

  start = clock::now();
  double r_b = midpoint_sum(v_b, v_a);
  end = clock::now();
  print_duration(start, end);

  // Use results
  cout << "Result: " << r_a << '\n';
  cout << "Result: " << r_b << '\n';
}

Original libc++:

1388 ms
21910 ms

Patched libc++:

1390 ms
1393 ms
mclow.lists accepted this revision.Dec 16 2019, 8:24 AM

Huh. I thought I fixed that typo. Anyway this LGTM.

This revision is now accepted and ready to land.Dec 16 2019, 8:24 AM

Please push, I don't have commit access.

This revision was automatically updated to reflect the committed changes.