Fixed style issues and shortened the phase name to help Golem understand it.
At this point it should be committable.


http://codereview.chromium.org/10032029/diff/14001/src/hydrogen.cc
File src/hydrogen.cc (right):

http://codereview.chromium.org/10032029/diff/14001/src/hydrogen.cc#newcode2639
src/hydrogen.cc:2639: int32_t Offset() {return offset_;}
On 2012/04/18 07:51:35, fschneider wrote:
Spaces around { and }.

Done.

http://codereview.chromium.org/10032029/diff/14001/src/hydrogen.cc#newcode2650
src/hydrogen.cc:2650: Representation::Integer32());
On 2012/04/18 07:51:35, fschneider wrote:
Indentation.

Done.

http://codereview.chromium.org/10032029/diff/14001/src/hydrogen.cc#newcode2694
src/hydrogen.cc:2694:
On 2012/04/18 07:51:35, fschneider wrote:
Two empty lines here.

Done.

http://codereview.chromium.org/10032029/diff/14001/src/hydrogen.cc#newcode2701
src/hydrogen.cc:2701:
On 2012/04/18 07:51:35, fschneider wrote:
Two lines also here (and in other places between functions and
classes)

Done.

http://codereview.chromium.org/10032029/diff/14001/src/hydrogen.cc#newcode2705
src/hydrogen.cc:2705: return
reinterpret_cast<BoundsCheckBbData**>(&(Lookup(key,
On 2012/04/18 07:51:35, fschneider wrote:
I'd break the line before the &. I think it will be better readable.

Done.

http://codereview.chromium.org/10032029/diff/14001/src/hydrogen.cc#newcode2738
src/hydrogen.cc:2738: // TODO(mmassi): allocate key only when we create
a new table  entry...
On 2012/04/18 07:51:35, fschneider wrote:
To solve this TODO you can maybe consider making BoundsCheckKey
BASE_EMBEDDED

We decided to leave the code as it is and remove the TODO.
This should cost us two pointers in the zone for every bounds check,
which is not *that* much.
The alternative is not hard to implement but the code would be less
clean, and the maintainability cost is likely higher than the memory
cost.

http://codereview.chromium.org/10032029/

--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev

Reply via email to