Skip to content

_BitScan{Reverse,Forward} add check for undefined#2031

Merged
bimbashrestha merged 3 commits intofacebook:devfrom
bimbashrestha:bitscan
Mar 10, 2020
Merged

_BitScan{Reverse,Forward} add check for undefined#2031
bimbashrestha merged 3 commits intofacebook:devfrom
bimbashrestha:bitscan

Conversation

@bimbashrestha
Copy link
Contributor

@bimbashrestha bimbashrestha commented Mar 5, 2020 •
edited
Loading

#1951

I've managed to reproduce this error on my windows vm, visual 2019 with:

#include <iostream>
#include <cassert>

using namespace std;

int main()
{
  unsigned long index = 0;
  _BitScanReverse64(&index, 0x0ull);

  cout << index << endl;

  assert(index == 0);

  return 0;
}

Silesia entire file benchmarks:

level zstd-clang (before) zstd-clang (after) zstd-gcc (before) zstd-gcc (after)
1 440 mb/s 440 mb/s (-) 454 mb/s 454 mb/s (-)
2 334 mb/s 337 mb/s (-) 334 mb/s 336 mb/s (-)
3 239 mb/s 243 mb/s (+1%) 241 mb/s 242 mb/s (-)
4 233 mb/s 230 mb/s (-1%) 226 mb/s 227 mb/s (-)
5 126 mb/s 128 mb/s (-) 126 mb/s 127 mb/s (-)
6 101 mb/s 101 mb/s (-) 102 mb/s 102 mb/s (-)
7 68 mb/s 67 mb/s (-) 72 mb/s 72 mb/s (-)
8 53 mb/s 54 mb/s (-) 58 mb/s 58 mb/s (-)
9 38 mb/s 38 mb/s (-) 42 mb/s 42 mb/s (-)
10 25 mb/s 25 mb/s (-) 27 mb/s 27 mb/s (-)
11 19 mb/s 19 mb/s (-) 19 mb/s 19 mb/s (-)
12 13 mb/s 14 mb/s (+4%) 13 mb/s 13 mb/s (-)
13 11.0 mb/s 10.95 mb/s (-) 10.99 mb/s 10.95 mb/s (-)
14 8.49 mb/s 8.44 mb/s (-) 8.16 mb/s 8.11 mb/s (-)
15 6.40 mb/s 6.34 mb/s (-) 6.24 mb/s 6.22 mb/s (-)
16 4.94 mb/s 4.92 mb/s (-) 5.15 mb/s 5.25 mb/s (-)
17 3.97 mb/s 3.95 mb/s (-) 4.06 mb/s 4.07 mb/s (-)
18 3.20 mb/s 3.20 mb/s (-) 3.31 mb/s 3.33 mb/s (-)
19 2.64 mb/s 2.66 mb/s (-) 2.77 mb/s 2.80 mb/s (-)

terrelln
terrelln reviewed Mar 7, 2020
View reviewed changes
Copy link
Contributor

@terrelln terrelln left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Except for the typo, the change looks good to me, just need the benchmarks.

@bimbashrestha
Copy link
Contributor Author

bimbashrestha commented Mar 10, 2020

Posted the benchmarks above. This seems pretty neutral to me for both gcc and clang.

@bimbashrestha bimbashrestha merged commit 5c05d41 into facebook:dev Mar 10, 2020
@bimbashrestha bimbashrestha deleted the bitscan branch March 17, 2020 13:42
@Cyan4973 Cyan4973 mentioned this pull request Mar 24, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@terrelln terrelln terrelln left review comments

@Cyan4973 Cyan4973 Cyan4973 approved these changes

Assignees

No one assigned

Labels

CLA Signed

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@bimbashrestha @Cyan4973 @terrelln @facebook-github-bot