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

Issue 62133002: Cleanup of branch code generation (no change in functionality). (Closed)

Created:
7 years, 1 month ago by fschneider
Modified:
7 years, 1 month ago
Reviewers:
zra, Florian Schneider
CC:
reviews_dartlang.org, vm-dev_dartlang.org
Visibility:
Public.

Description

Cleanup of branch code generation (no change in functionality). This CL is the first step in refactoring the way branches and comparisons are generated. 1. Move helper functions from flow_graph_compiler_xyz.cc to intermediate_language_xyz.cc. 2. Remove IL class ControlInstruction. It was only implemented by BranchInstr. All functions provided are moved to BranchInstr. 3. When generating branch code for comparisons, pass the successor labels explicitly instead of getting them from the branch. This will allow us to provide different labels when materialize a bool value of a comparison. 4. Move some common code for IfThenElseInstr from the platform-specific files into intermediate_language.cc and simplify it. The goal is to enable if-conversion of arbitrary comparisons. Right now, the code for == is hard-coded in IfThenElseInstr (and duplicated, too). This means that e.g. "a < b ? 0 : 1" cannot be optimized. As a result, IfThenElseInstr can be used to materialize the boolean value of a comparison. This way the complication of having ComparisonInstr both as a normal instruction and as wrapped inside a BranchInstr can be simplified. Comparisons would no longer appear as plain instructions in the IL, but only wrapped inside either a Branch or an IfThenElse(true, false). R=zra@google.com Committed: https://code.google.com/p/dart/source/detail?r=30042

Patch Set 1 #

Patch Set 2 : #

Patch Set 3 : rebased #

Unified diffs Side-by-side diffs Delta from patch set Stats (+469 lines, -537 lines) Patch
M runtime/vm/flow_graph_compiler.h View 1 2 1 chunk +0 lines, -9 lines 0 comments Download
M runtime/vm/flow_graph_compiler_arm.cc View 1 2 1 chunk +0 lines, -34 lines 0 comments Download
M runtime/vm/flow_graph_compiler_ia32.cc View 1 2 1 chunk +0 lines, -33 lines 0 comments Download
M runtime/vm/flow_graph_compiler_mips.cc View 1 2 1 chunk +0 lines, -76 lines 0 comments Download
M runtime/vm/flow_graph_compiler_x64.cc View 1 2 1 chunk +0 lines, -32 lines 0 comments Download
M runtime/vm/intermediate_language.h View 1 2 6 chunks +13 lines, -37 lines 0 comments Download
M runtime/vm/intermediate_language.cc View 1 2 3 chunks +26 lines, -3 lines 0 comments Download
M runtime/vm/intermediate_language_arm.cc View 1 2 10 chunks +90 lines, -72 lines 0 comments Download
M runtime/vm/intermediate_language_ia32.cc View 1 2 11 chunks +116 lines, -93 lines 0 comments Download
M runtime/vm/intermediate_language_mips.cc View 1 2 10 chunks +136 lines, -76 lines 0 comments Download
M runtime/vm/intermediate_language_x64.cc View 1 2 10 chunks +88 lines, -72 lines 0 comments Download

Messages

Total messages: 3 (0 generated)
Florian Schneider
7 years, 1 month ago (2013-11-06 15:56:42 UTC) #1
zra
lgtm
7 years, 1 month ago (2013-11-06 17:06:38 UTC) #2
fschneider
7 years, 1 month ago (2013-11-07 11:47:18 UTC) #3
Message was sent while issue was closed.
Committed patchset #3 manually as r30042 (presubmit successful).

Powered by Google App Engine
This is Rietveld 408576698