Reviewers: William Hesse,

http://codereview.chromium.org/570002/diff/1/2
File src/codegen.cc (right):

http://codereview.chromium.org/570002/diff/1/2#newcode485
src/codegen.cc:485: const bool indirect_result = result_size_ > 1;
On 2010/02/03 06:18:35, William Hesse wrote:
This seems really wrong.  Either make the type (int) explicit, or make
this a
constant bool.

It supposed to be bool. Fixed.

http://codereview.chromium.org/570002/diff/1/2#newcode491
src/codegen.cc:491: | IndirectResultBits::encode(indirect_result);
On 2010/02/03 06:18:35, William Hesse wrote:
Shouldn't this just be indirect_result, not indirect_result > 1?  The
second
expression is never true.

Sure, it must.

In all cases except _WIN64, this is encoding 0, which probably doesn't
affect
the bitwise or, so maybe the entire return statement should be in an
#ifdef, and
be optimized in all cases except WIN64 to just return
ExitFrameModeBits::encode(mode).

I don't think such a manual optimization would make sense since it's a
clear case for the C++ optimizer.



Please review this at http://codereview.chromium.org/570002

SVN Base: http://v8.googlecode.com/svn/branches/bleeding_edge/

Affected files:
  M     src/codegen.cc


Index: src/codegen.cc
===================================================================
--- src/codegen.cc      (revision 3775)
+++ src/codegen.cc      (working copy)
@@ -482,13 +482,13 @@
 int CEntryStub::MinorKey() {
   ASSERT(result_size_ <= 2);
 #ifdef _WIN64
-  const indirect_result = result_size_ > 1;
+  const bool indirect_result = result_size_ > 1;
 #else
   const bool indirect_result = false;
 #endif

   return ExitFrameModeBits::encode(mode_)
-         | IndirectResultBits::encode(indirect_result > 1);
+         | IndirectResultBits::encode(indirect_result);
 }




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

Reply via email to