Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(151)

Issue 867383002: Implement bitLength intrinsic on x64. (Closed)

Created:
5 years, 11 months ago by srdjan
Modified:
5 years, 11 months ago
CC:
reviews_dartlang.org, vm-dev_dartlang.org
Visibility:
Public.

Description

Patch Set 1 #

Total comments: 3
Unified diffs Side-by-side diffs Delta from patch set Stats (+57 lines, -8 lines) Patch
M runtime/vm/assembler_x64.h View 1 chunk +2 lines, -0 lines 0 comments Download
M runtime/vm/assembler_x64.cc View 1 chunk +11 lines, -0 lines 0 comments Download
M runtime/vm/assembler_x64_test.cc View 2 chunks +26 lines, -3 lines 1 comment Download
M runtime/vm/disassembler_x64.cc View 2 chunks +6 lines, -4 lines 0 comments Download
M runtime/vm/intrinsifier_x64.cc View 1 chunk +12 lines, -1 line 2 comments Download

Messages

Total messages: 8 (3 generated)
srdjan
5 years, 11 months ago (2015-01-23 21:30:49 UTC) #2
Cutch
lgtm https://codereview.chromium.org/867383002/diff/1/runtime/vm/assembler_x64_test.cc File runtime/vm/assembler_x64_test.cc (right): https://codereview.chromium.org/867383002/diff/1/runtime/vm/assembler_x64_test.cc#newcode623 runtime/vm/assembler_x64_test.cc:623: __ movq(RCX, Immediate(42)); Good catch.
5 years, 11 months ago (2015-01-23 22:10:00 UTC) #4
srdjan
Committed patchset #1 (id:1) manually as r43126 (presubmit successful).
5 years, 11 months ago (2015-01-23 22:41:11 UTC) #5
Lasse Reichstein Nielsen
https://codereview.chromium.org/867383002/diff/1/runtime/vm/intrinsifier_x64.cc File runtime/vm/intrinsifier_x64.cc (right): https://codereview.chromium.org/867383002/diff/1/runtime/vm/intrinsifier_x64.cc#newcode801 runtime/vm/intrinsifier_x64.cc:801: __ sarq(RCX, Immediate(63)); // All 0 or all 1. ...
5 years, 11 months ago (2015-01-25 11:26:53 UTC) #7
srdjan
5 years, 11 months ago (2015-01-26 18:43:18 UTC) #8
Message was sent while issue was closed.
https://codereview.chromium.org/867383002/diff/1/runtime/vm/intrinsifier_x64.cc
File runtime/vm/intrinsifier_x64.cc (right):

https://codereview.chromium.org/867383002/diff/1/runtime/vm/intrinsifier_x64....
runtime/vm/intrinsifier_x64.cc:801: __ sarq(RCX, Immediate(63));  // All 0 or
all 1.
On 2015/01/25 11:26:53, Lasse Reichstein Nielsen wrote:
> If you shift RAX instead of RCX here, would the dependecy depth drop by one?

Thanks for the comment. In theory yes, but with Intel's register renaming, it
should not matter, AFAIK.
Will change it once I tackle MIPS and ARM implementations as well.

Powered by Google App Engine
This is Rietveld 408576698