Thanks Erik for review, updated as per comments. Added a test. Please
review.
http://codereview.chromium.org/6170001/diff/55003/src/arm/macro-assembler-arm.cc
File src/arm/macro-assembler-arm.cc (right):
http://codereview.chromium.org/6170001/diff/55003/src/arm/macro-assembler-arm.cc#newcode1398
src/arm/macro-assembler-arm.cc:1398:
On 2011/01/24 21:45:16, Erik Corry wrote:
You should have 2 blank lines between functions.
Done.
http://codereview.chromium.org/6170001/diff/55003/src/arm/simulator-arm.cc
File src/arm/simulator-arm.cc (right):
http://codereview.chromium.org/6170001/diff/55003/src/arm/simulator-arm.cc#newcode1600
src/arm/simulator-arm.cc:1600: } else {
On 2011/01/24 21:45:16, Erik Corry wrote:
The usual pattern in V8 is to add a line here saying something like.
ASSERT(redirection->type() == ExternalReference::...)
which serves as a check and also a comment that aids the reader.
Done.
http://codereview.chromium.org/6170001/diff/55003/src/arm/simulator-arm.h
File src/arm/simulator-arm.h (right):
http://codereview.chromium.org/6170001/diff/55003/src/arm/simulator-arm.h#newcode289
src/arm/simulator-arm.h:289: v8::internal::ExternalReference::Type
type);
On 2011/01/24 21:45:16, Erik Corry wrote:
The indentation of these two parameters is wrong.
The second parameter is too long for 80chars. moved both to next lines
and aligned it.
http://codereview.chromium.org/6170001/diff/55003/src/arm/stub-cache-arm.cc
File src/arm/stub-cache-arm.cc (right):
http://codereview.chromium.org/6170001/diff/55003/src/arm/stub-cache-arm.cc#newcode595
src/arm/stub-cache-arm.cc:595: static bool
GenerateFastApiDirectCall(MacroAssembler* masm,
On 2011/01/24 21:45:16, Erik Corry wrote:
Instead of returning a bool and using a Failure** to return the
failure this
function should return a MaybeObject*. This is the pattern used
throughout V8
for functions that may encounter an allocation failure. You can
return
Heap::undefined_value() in the success cases where there is nothing to
return.
Actually i followed the IA32 return structure. Modified as per
suggestion.
Done.
http://codereview.chromium.org/6170001/
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
