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

Issue 1279833005: Inline memcpy for small constant sizes. (Closed)

Created:
5 years, 4 months ago by ascull
Modified:
5 years, 4 months ago
CC:
native-client-reviews_googlegroups.com
Base URL:
https://chromium.googlesource.com/native_client/pnacl-subzero.git@master
Target Ref:
refs/heads/master
Visibility:
Public.

Description

Inline memcpy for small constant sizes. Combined with memset inlining, this has shown an improvement of over 11% on the eon benchmark. This the only C++ program in spec2k and it seems C++ programs have a significantly larger number of memset/memcpy calls. Other benchmarks also showed improvement of ~5% (perlbmk, parser) while most had a 1-2% improvement. This commit also includes a refactoring of lowerMemset which is much more readable and also removed the fake use of the destination pointer register. BUG= R=jvoung@chromium.org, stichnot@chromium.org Committed: https://gerrit.chromium.org/gerrit/gitweb?p=native_client/pnacl-subzero.git;a=commit;h=9df4a379ebdb2f2200ec7f65017bd9fc8544794b

Patch Set 1 #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+283 lines, -94 lines) Patch
M src/IceTargetLoweringX86Base.h View 1 chunk +3 lines, -1 line 0 comments Download
M src/IceTargetLoweringX86BaseImpl.h View 3 chunks +172 lines, -87 lines 0 comments Download
M tests_lit/llvm2ice_tests/nacl-mem-intrinsics.ll View 1 chunk +108 lines, -6 lines 2 comments Download

Messages

Total messages: 5 (1 generated)
ascull
5 years, 4 months ago (2015-08-06 22:28:02 UTC) #2
Jim Stichnoth
LGTM, though it looks like there's a lot of commonality between memset and memcpy. Once ...
5 years, 4 months ago (2015-08-07 15:13:32 UTC) #3
jvoung (off chromium)
LGTM too https://codereview.chromium.org/1279833005/diff/1/tests_lit/llvm2ice_tests/nacl-mem-intrinsics.ll File tests_lit/llvm2ice_tests/nacl-mem-intrinsics.ll (right): https://codereview.chromium.org/1279833005/diff/1/tests_lit/llvm2ice_tests/nacl-mem-intrinsics.ll#newcode148 tests_lit/llvm2ice_tests/nacl-mem-intrinsics.ll:148: ; TODO(jvoung) -- if we want to ...
5 years, 4 months ago (2015-08-07 15:37:08 UTC) #4
ascull
5 years, 4 months ago (2015-08-07 16:19:39 UTC) #5
Message was sent while issue was closed.
Committed patchset #1 (id:1) manually as
9df4a379ebdb2f2200ec7f65017bd9fc8544794b (presubmit successful).

Powered by Google App Engine
This is Rietveld 408576698