[v8] [EMAIL PROTECTED] commented on revision r949.
Details are at http://code.google.com/p/v8/source/detail?r=949
Score: Neutral
General Comment:
Bill,
I am lacking a way to understand how you would use this Result class.
Ideally the code generator could deal with the frame elements directly so
that you do not have to add an abstraction on top of an abstraction. In
this case you are actually loosing information doing it.
-Ivan
Line-by-line comments:
File: /branches/experimental/toiger/src/virtual-frame-ia32.cc (r949)
===============================================================================
Line 53: ASSERT(!reg().is(no_reg));
-------------------------------------------------------------------------------
What happens if you Unuse() a non-register Result?
File: /branches/experimental/toiger/src/virtual-frame-ia32.h (r949)
===============================================================================
Line 40: class Result BASE_EMBEDDED {
-------------------------------------------------------------------------------
Result as a name is too generic. Also I do not understand the distinction
between a VirtualFrame element and the Result here. Why can't the code
generator not just Pop() the virtual frame element? What if the top of the
stack is a memory address, the code generator is just fine dealing with
stack references especially on IA32.
Line 84: CodeGenerator* cgen_;
-------------------------------------------------------------------------------
In any case the DISALLOW_COPY_AND_ASSIGN macro is missing here.
Respond to these comments at http://code.google.com/p/v8/source/detail?r=949
--
You received this message because you starred this review, or because
your project has directed all notifications to a mailing list that you
subscribe to.
You may adjust your review notification preferences at:
http://code.google.com/hosting/settings
--~--~---------~--~----~------------~-------~--~----~
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
-~----------~----~----~----~------~----~------~--~---
