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

Issue 169893002: A64: Let the MacroAssembler resolve branches to distant targets. (Closed)

Created:
6 years, 10 months ago by Alexandre Rames
Modified:
6 years, 10 months ago
CC:
v8-dev
Visibility:
Public.

Description

A64: Let the MacroAssembler resolve branches to distant targets. Code generation would fail when assembling a branch to a label that is bound outside the immediate range of the instruction. A64 is sensitive to this, as the various branching instructions have different ranges, going down to +-32KB for TBZ/TBNZ. The MacroAssembler is augmented to handle branches to targets that may exceed the immediate range of instructions. When branching backward to a label exceeding the instruction range, the MacroAssembler can simply tweak the generated code to use an unconditional branch with a longer range. For example instead of B(cond, &label); the MacroAssembler can generate: b(InvertCondition(cond), &done); b(&label); bind(&done); Since the target is not known when the branch is emitted, forward branches uses a different mechanism. The MacroAssembler keeps track of forward branches to unbound labels. When the code generation approaches the end of the range of a branch, a veneer is generated for the branch. BUG=v8:3148 LOG=Y R=ulan@chromium.org Committed: https://code.google.com/p/v8/source/detail?r=19444

Patch Set 1 #

Total comments: 8

Patch Set 2 : Addressed Ulan's comments. #

Total comments: 1
Unified diffs Side-by-side diffs Delta from patch set Stats (+782 lines, -68 lines) Patch
M src/a64/assembler-a64.h View 4 chunks +42 lines, -0 lines 0 comments Download
M src/a64/assembler-a64.cc View 1 6 chunks +101 lines, -5 lines 0 comments Download
M src/a64/assembler-a64-inl.h View 1 chunk +10 lines, -0 lines 0 comments Download
M src/a64/instructions-a64.h View 2 chunks +25 lines, -0 lines 0 comments Download
M src/a64/instructions-a64.cc View 1 chunk +12 lines, -0 lines 0 comments Download
M src/a64/macro-assembler-a64.h View 6 chunks +61 lines, -17 lines 0 comments Download
M src/a64/macro-assembler-a64.cc View 1 1 chunk +198 lines, -0 lines 0 comments Download
M src/a64/macro-assembler-a64-inl.h View 6 chunks +2 lines, -37 lines 0 comments Download
M test/cctest/test-assembler-a64.cc View 1 chunk +329 lines, -0 lines 0 comments Download
M test/mjsunit/mjsunit.status View 1 chunk +2 lines, -3 lines 1 comment Download
M test/mozilla/mozilla.status View 1 chunk +0 lines, -6 lines 0 comments Download

Messages

Total messages: 7 (0 generated)
Alexandre Rames
I have had some trouble with git cl uploading a 2nd version of this patch, ...
6 years, 10 months ago (2014-02-17 18:18:25 UTC) #1
ulan
Looks good overall! Few comments below: https://codereview.chromium.org/169893002/diff/1/src/a64/assembler-a64.cc File src/a64/assembler-a64.cc (right): https://codereview.chromium.org/169893002/diff/1/src/a64/assembler-a64.cc#newcode388 src/a64/assembler-a64.cc:388: // The branch ...
6 years, 10 months ago (2014-02-18 10:17:40 UTC) #2
Alexandre Rames
https://codereview.chromium.org/169893002/diff/1/src/a64/assembler-a64.cc File src/a64/assembler-a64.cc (right): https://codereview.chromium.org/169893002/diff/1/src/a64/assembler-a64.cc#newcode388 src/a64/assembler-a64.cc:388: // The branch is the first instruction in the ...
6 years, 10 months ago (2014-02-18 11:59:20 UTC) #3
ulan
Thanks, lgtm!
6 years, 10 months ago (2014-02-18 12:14:00 UTC) #4
ulan
https://codereview.chromium.org/169893002/diff/100001/test/mjsunit/mjsunit.status File test/mjsunit/mjsunit.status (right): https://codereview.chromium.org/169893002/diff/100001/test/mjsunit/mjsunit.status#newcode180 test/mjsunit/mjsunit.status:180: 'sin-cos': [PASS, ['mode == debug', FAIL]], Jochen noticed that ...
6 years, 10 months ago (2014-02-18 12:22:02 UTC) #5
Alexandre Rames
On 2014/02/18 12:22:02, ulan wrote: > https://codereview.chromium.org/169893002/diff/100001/test/mjsunit/mjsunit.status > File test/mjsunit/mjsunit.status (right): > > https://codereview.chromium.org/169893002/diff/100001/test/mjsunit/mjsunit.status#newcode180 > ...
6 years, 10 months ago (2014-02-18 13:09:23 UTC) #6
Alexandre Rames
6 years, 10 months ago (2014-02-18 13:15:43 UTC) #7
Message was sent while issue was closed.
Committed patchset #2 manually as r19444 (presubmit successful).

Powered by Google App Engine
This is Rietveld 408576698