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

Issue 410333003: Shorter TryAllocate instruction sequence on ARM/ARM64/MIPS. (Closed)

Created:
6 years, 5 months ago by Vyacheslav Egorov (Google)
Modified:
6 years, 5 months ago
Reviewers:
regis, Cutch
CC:
reviews_dartlang.org, vm-dev_dartlang.org
Visibility:
Public.

Description

Use shorter TryAllocate instruction sequence on ARM/ARM64/MIPS. On these architectures it's preferable to load Scavenger address into the register and then access top/end fields through this register instead of loading addresses of those fields as immediates --- which takes either two instructions or a memory load through a pool pointer. Cleanup boxing slow-paths in the optimizing compiler. We had tons of duplicated code that was essentially doing the same thing. BUG= R=johnmccutchan@google.com, regis@google.com Committed: https://code.google.com/p/dart/source/detail?r=38539

Patch Set 1 #

Patch Set 2 : #

Total comments: 4
Unified diffs Side-by-side diffs Delta from patch set Stats (+542 lines, -1341 lines) Patch
M runtime/vm/assembler.cc View 1 2 chunks +6 lines, -1 line 0 comments Download
M runtime/vm/assembler_arm.h View 1 chunk +1 line, -0 lines 0 comments Download
M runtime/vm/assembler_arm.cc View 1 chunk +8 lines, -7 lines 0 comments Download
M runtime/vm/assembler_arm64.h View 2 chunks +2 lines, -0 lines 1 comment Download
M runtime/vm/assembler_arm64.cc View 1 chunk +5 lines, -6 lines 3 comments Download
M runtime/vm/assembler_ia32.h View 1 chunk +2 lines, -0 lines 0 comments Download
M runtime/vm/assembler_mips.h View 1 chunk +1 line, -0 lines 0 comments Download
M runtime/vm/assembler_mips.cc View 1 chunk +5 lines, -6 lines 0 comments Download
M runtime/vm/assembler_x64.h View 1 chunk +2 lines, -0 lines 0 comments Download
M runtime/vm/flow_graph_compiler.h View 2 chunks +2 lines, -0 lines 0 comments Download
M runtime/vm/flow_graph_compiler.cc View 1 chunk +2 lines, -0 lines 0 comments Download
M runtime/vm/heap.h View 1 chunk +1 line, -0 lines 0 comments Download
M runtime/vm/intermediate_language_arm.cc View 18 chunks +128 lines, -316 lines 0 comments Download
M runtime/vm/intermediate_language_arm64.cc View 14 chunks +110 lines, -272 lines 0 comments Download
M runtime/vm/intermediate_language_ia32.cc View 19 chunks +103 lines, -351 lines 0 comments Download
M runtime/vm/intermediate_language_mips.cc View 7 chunks +64 lines, -89 lines 0 comments Download
M runtime/vm/intermediate_language_x64.cc View 14 chunks +95 lines, -289 lines 0 comments Download
M runtime/vm/intrinsifier_arm64.cc View 5 chunks +5 lines, -4 lines 0 comments Download

Messages

Total messages: 6 (0 generated)
Vyacheslav Egorov (Google)
PTAL
6 years, 5 months ago (2014-07-23 21:55:30 UTC) #1
Cutch
lgtm. Nice cleanup!
6 years, 5 months ago (2014-07-23 21:59:58 UTC) #2
regis
LGTM Nice! https://codereview.chromium.org/410333003/diff/20001/runtime/vm/assembler_arm64.cc File runtime/vm/assembler_arm64.cc (right): https://codereview.chromium.org/410333003/diff/20001/runtime/vm/assembler_arm64.cc#newcode1379 runtime/vm/assembler_arm64.cc:1379: Register temp_reg, Use TMP2. https://codereview.chromium.org/410333003/diff/20001/runtime/vm/assembler_arm64.h File runtime/vm/assembler_arm64.h ...
6 years, 5 months ago (2014-07-23 22:29:33 UTC) #3
Vyacheslav Egorov (Google)
https://codereview.chromium.org/410333003/diff/20001/runtime/vm/assembler_arm64.cc File runtime/vm/assembler_arm64.cc (right): https://codereview.chromium.org/410333003/diff/20001/runtime/vm/assembler_arm64.cc#newcode1379 runtime/vm/assembler_arm64.cc:1379: Register temp_reg, On 2014/07/23 22:29:33, regis wrote: > Use ...
6 years, 5 months ago (2014-07-24 11:24:49 UTC) #4
Vyacheslav Egorov (Google)
Committed patchset #2 manually as r38539 (presubmit successful).
6 years, 5 months ago (2014-07-24 12:04:37 UTC) #5
regis
6 years, 5 months ago (2014-07-24 17:06:07 UTC) #6
Message was sent while issue was closed.
https://codereview.chromium.org/410333003/diff/20001/runtime/vm/assembler_arm...
File runtime/vm/assembler_arm64.cc (right):

https://codereview.chromium.org/410333003/diff/20001/runtime/vm/assembler_arm...
runtime/vm/assembler_arm64.cc:1379: Register temp_reg,
On 2014/07/24 11:24:48, Vyacheslav Egorov (Google wrote:
> On 2014/07/23 22:29:33, regis wrote:
> > Use TMP2.
> 
> AddImmediate is using TMP2 as a scratch register on some code paths, so while
> the code path that uses TMP2 is unlikely to be hit I can't strictly speaking
use
> TMP2 here.

Acknowledged.

Powered by Google App Engine
This is Rietveld 408576698