Issue 10806078: Improve the performance of bit set searches with a compiler intrinsic.

Issue 10806078: Improve the performance of bit set searches with a compiler intrinsic. (Closed)

Created:
8 years, 5 months ago by cshapiro

Modified:
8 years, 5 months ago

Reviewers:
siva

CC:
reviews_dartlang.org, vm-dev_dartlang.org

Base URL:
https://dart.googlecode.com/svn/branches/bleeding_edge/dart

Visibility:
Public.

More Reviews

Description

Improve the performance of bit set searches with a compiler intrinsic. Committed: https://code.google.com/p/dart/source/detail?r=9836

Patch Set 2 : add missing include #

Patch Set 3 : add new header files #

Total comments: 3

Patch Set 4 : address review comments #

Created: 8 years, 5 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+84 lines, -40 lines)			Patch
M	runtime/platform/utils.h	View		2 chunks	+13 lines, -0 lines	0 comments	Download
A +	runtime/platform/utils_linux.h	View	1 2	1 chunk	+8 lines, -13 lines	0 comments	Download
A +	runtime/platform/utils_macos.h	View	1 2	1 chunk	+8 lines, -13 lines	0 comments	Download
A	runtime/platform/utils_win.h	View	1 2	1 chunk	+26 lines, -0 lines	0 comments	Download
M	runtime/vm/bit_set.h	View	1	2 chunks	+18 lines, -0 lines	0 comments	Download
M	runtime/vm/freelist.h	View	1 2 3	1 chunk	+1 line, -1 line	0 comments	Download
M	runtime/vm/freelist.cc	View	1 2 3	3 chunks	+10 lines, -13 lines	0 comments	Download

Messages

Total messages: 3 (0 generated)

Expand Messages | Collapse Messages

siva

LGTM with couple of comments. http://codereview.chromium.org/10806078/diff/5001/runtime/platform/utils.h File runtime/platform/utils.h (right): http://codereview.chromium.org/10806078/diff/5001/runtime/platform/utils.h#newcode83 runtime/platform/utils.h:83: static int CountTrailingZeros(uint64_t x); ...

8 years, 5 months ago (2012-07-24 01:07:43 UTC) #2

cshapiro

8 years, 5 months ago (2012-07-24 02:43:50 UTC) #3

http://codereview.chromium.org/10806078/diff/5001/runtime/platform/utils.h
File runtime/platform/utils.h (right):

http://codereview.chromium.org/10806078/diff/5001/runtime/platform/utils.h#ne...
runtime/platform/utils.h:83: static int CountTrailingZeros(uint64_t x);
Originally, I wrote this method as a template like so

template<typename T>
int CountTrailingZeros(T x) {
  if (sizeof(x) == 4) // 32-bit intrinsic
  else if (sizeof(x) == 8) // 64 bit intrinsic
  else // assert
}

This looked a bit ugly and seemed like overkill so I just punted on it and
settled on separate versions for 32-bit and 64-bit because the other two bit
operations on lines 79 and 80 were specified for 32-bit words and not a uword.

However, this might just be shortsightedness and maybe all of these functions
should be defined for uword only.  Let me know what you think and I will happily
fix this and do a cleanup for the other 2 definitions as well.

Expand Messages | Collapse Messages