Reduce the number of virtual functions in hydrogen-instruction.h classes

Reduce the number of virtual function in hydrogen-instruction.h classes

Instead of using virtual type-tester functions we can just
generate non-virtual ones for all concrete IR classes.

This is changes reduces the V8 binary size by ~2%.

I also simplified the macros to declare new hydrogen instructions slightly.
The name used for debug output is no longer passed as a separate string.
Instead we just use the class name.
Committed: http://code.google.com/p/v8/source/detail?r=7659

[Kevin Millikin (Chromium), 2011-04-19 07:40:52]

Comments below.

http://codereview.chromium.org/6880014/diff/1/src/hydrogen-instructions.h
File src/hydrogen-instructions.h (right):

http://codereview.chromium.org/6880014/diff/1/src/hydrogen-instructions.h#new...
src/hydrogen-instructions.h:134: V(Phi)                                       \
It seems like a type error to have Phi (a Value but not an Instruction) be a
member of the instruction list.

Can we remove it and explicitly handle it only where needed?  Then we could also
get rid of stuff that doesn't make sense like the unreachable
LChunkBuilder::DoPhi.

Otherwise, can we rename to CONCRETE_VALUE and ABSTRACT_VALUE?

http://codereview.chromium.org/6880014/diff/1/src/hydrogen-instructions.h#new...
src/hydrogen-instructions.h:179: virtual bool Is##type() const { return true; } 
  \
I'd prefer if we could come up with a way to have the opcode() function virtual
and the IsXXX functions non-virtual instead.

opcode() was introduced mostly for the purpose of inquiring in the debugger what
the concrete type of a random HValue is.

http://codereview.chromium.org/6880014/diff/1/src/hydrogen-instructions.h#new...
src/hydrogen-instructions.h:454: #define DECLARE_DO(type) k##type,
This is not part of your change, but I really dislike seeing "DECLARE_DO" for
something that is not declaring a DoXXX function at all, just because it was
copy-pasted from somewhere else.

Could you change this to "DECLARE_OPCODE" or something that makes sense at this
use?

http://codereview.chromium.org/6880014/diff/1/src/hydrogen-instructions.h#new...
src/hydrogen-instructions.h:457: kMaxInstructionClass
This seems like another silly name.  I don't know what it means to be the
"maximum" class.  I do know what kInstructionCount or (less good)
kNumberOfInstructions means.

http://codereview.chromium.org/6880014/diff/1/src/hydrogen-instructions.h#new...
src/hydrogen-instructions.h:462: #define DECLARE_DO(type) bool Is##type() const
{ return opcode() == k##type; }
Please DECLARE_PREDICATE or something.

Do we ever need a virtual IsXXX function on concrete classes if we have the
non-virtual one on the base class?

http://codereview.chromium.org/6880014/diff/1/src/hydrogen-instructions.h#new...
src/hydrogen-instructions.h:467: #define DECLARE_DO(type) virtual bool
Is##type() const { return false; }
Same comment.

http://codereview.chromium.org/6880014/diff/1/src/hydrogen-instructions.h#new...
src/hydrogen-instructions.h:769: DECLARE_CONCRETE_INSTRUCTION(BlockEntry)
Thank you.

[fschneider, 2011-04-19 09:08:54]

http://codereview.chromium.org/6880014/diff/1/src/hydrogen-instructions.h
File src/hydrogen-instructions.h (right):

http://codereview.chromium.org/6880014/diff/1/src/hydrogen-instructions.h#new...
src/hydrogen-instructions.h:134: V(Phi)                                       \
On 2011/04/19 07:40:52, Kevin Millikin wrote:
> It seems like a type error to have Phi (a Value but not an Instruction) be a
> member of the instruction list.
> 
> Can we remove it and explicitly handle it only where needed?  Then we could
also
> get rid of stuff that doesn't make sense like the unreachable
> LChunkBuilder::DoPhi.
> 
> Otherwise, can we rename to CONCRETE_VALUE and ABSTRACT_VALUE?

Done.

http://codereview.chromium.org/6880014/diff/1/src/hydrogen-instructions.h#new...
src/hydrogen-instructions.h:179: virtual bool Is##type() const { return true; } 
  \
On 2011/04/19 07:40:52, Kevin Millikin wrote:
> I'd prefer if we could come up with a way to have the opcode() function
virtual
> and the IsXXX functions non-virtual instead.
> 
> opcode() was introduced mostly for the purpose of inquiring in the debugger
what
> the concrete type of a random HValue is.

Done.

http://codereview.chromium.org/6880014/diff/1/src/hydrogen-instructions.h#new...
src/hydrogen-instructions.h:454: #define DECLARE_DO(type) k##type,
On 2011/04/19 07:40:52, Kevin Millikin wrote:
> This is not part of your change, but I really dislike seeing "DECLARE_DO" for
> something that is not declaring a DoXXX function at all, just because it was
> copy-pasted from somewhere else.
> 
> Could you change this to "DECLARE_OPCODE" or something that makes sense at
this
> use?

Done.

http://codereview.chromium.org/6880014/diff/1/src/hydrogen-instructions.h#new...
src/hydrogen-instructions.h:457: kMaxInstructionClass
On 2011/04/19 07:40:52, Kevin Millikin wrote:
> This seems like another silly name.  I don't know what it means to be the
> "maximum" class.  I do know what kInstructionCount or (less good)
> kNumberOfInstructions means.

Done.

http://codereview.chromium.org/6880014/diff/1/src/hydrogen-instructions.h#new...
src/hydrogen-instructions.h:462: #define DECLARE_DO(type) bool Is##type() const
{ return opcode() == k##type; }
On 2011/04/19 07:40:52, Kevin Millikin wrote:
> Please DECLARE_PREDICATE or something.
> 
> Do we ever need a virtual IsXXX function on concrete classes if we have the
> non-virtual one on the base class?

Done.

http://codereview.chromium.org/6880014/diff/1/src/hydrogen-instructions.h#new...
src/hydrogen-instructions.h:467: #define DECLARE_DO(type) virtual bool
Is##type() const { return false; }
On 2011/04/19 07:40:52, Kevin Millikin wrote:
> Same comment.

Done.