Improve instruction creating/adding shorthand in HGraphBuilder

Improve instruction creating/adding shorthand in HGraphBuilder

Add multi-argument templates for New, NewUncasted, Add and AddUncasted that
call boilerplate HInstruction::New methods rather than the constructor directly.
This allows for automatic passing of the zone and context for instructions that
need them.

R=verwaest@chromium.org

Committed: http://code.google.com/p/v8/source/detail?r=15990

[Toon Verwaest, 2013-07-31 12:56:36]

Mostly nits.

I'd prefer to have AddAndCast be renamed as the default Add behaviour, and
rename Add to AddUncasted.

Otherwise looks good.

https://chromiumcodereview.appspot.com/21356002/diff/1/src/hydrogen-instructi...
File src/hydrogen-instructions.cc (right):

https://chromiumcodereview.appspot.com/21356002/diff/1/src/hydrogen-instructi...
src/hydrogen-instructions.cc:4316: HConstant::New(graph->zone(), NULL,
graph->GetInvalidContext()

https://chromiumcodereview.appspot.com/21356002/diff/1/src/hydrogen-instructi...
File src/hydrogen-instructions.h (right):

https://chromiumcodereview.appspot.com/21356002/diff/1/src/hydrogen-instructi...
src/hydrogen-instructions.h:1333: HValue* context,                              
         \
Weird indentation.

https://chromiumcodereview.appspot.com/21356002/diff/1/src/hydrogen-instructi...
src/hydrogen-instructions.h:1342: static I* New(Zone* zone,                     
                              \
Weird indentation.

https://chromiumcodereview.appspot.com/21356002/diff/1/src/hydrogen-instructi...
src/hydrogen-instructions.h:4416: // control flow instructions.lithium
Left-over "lithium" at the end of the sentence.

https://chromiumcodereview.appspot.com/21356002/diff/1/src/hydrogen.h
File src/hydrogen.h (right):

https://chromiumcodereview.appspot.com/21356002/diff/1/src/hydrogen.h#newcode...
src/hydrogen.h:1511: HValue* BuildKeyHash(HValue* key);
Left-over declaration from a different CL?

[danno, 2013-07-31 14:10:09]

Please take another look

https://chromiumcodereview.appspot.com/21356002/diff/1/src/hydrogen-instructi...
File src/hydrogen-instructions.cc (right):

https://chromiumcodereview.appspot.com/21356002/diff/1/src/hydrogen-instructi...
src/hydrogen-instructions.cc:4316: HConstant::New(graph->zone(), NULL,
On 2013/07/31 12:56:37, Toon Verwaest wrote:
> graph->GetInvalidContext()

Done.

https://chromiumcodereview.appspot.com/21356002/diff/1/src/hydrogen-instructi...
File src/hydrogen-instructions.h (right):

https://chromiumcodereview.appspot.com/21356002/diff/1/src/hydrogen-instructi...
src/hydrogen-instructions.h:1333: HValue* context,                              
         \
On 2013/07/31 12:56:37, Toon Verwaest wrote:
> Weird indentation.

Done.

https://chromiumcodereview.appspot.com/21356002/diff/1/src/hydrogen-instructi...
src/hydrogen-instructions.h:1342: static I* New(Zone* zone,                     
                              \
On 2013/07/31 12:56:37, Toon Verwaest wrote:
> Weird indentation.

Done.

https://chromiumcodereview.appspot.com/21356002/diff/1/src/hydrogen-instructi...
src/hydrogen-instructions.h:4416: // control flow instructions.lithium
On 2013/07/31 12:56:37, Toon Verwaest wrote:
> Left-over "lithium" at the end of the sentence.

Done.

https://chromiumcodereview.appspot.com/21356002/diff/1/src/hydrogen.h
File src/hydrogen.h (right):

https://chromiumcodereview.appspot.com/21356002/diff/1/src/hydrogen.h#newcode...
src/hydrogen.h:1511: HValue* BuildKeyHash(HValue* key);
On 2013/07/31 12:56:37, Toon Verwaest wrote:
> Left-over declaration from a different CL?

Done.

[Toon Verwaest, 2013-07-31 14:32:53]

Added comments.

https://chromiumcodereview.appspot.com/21356002/diff/18001/src/hydrogen.cc
File src/hydrogen.cc (right):

https://chromiumcodereview.appspot.com/21356002/diff/18001/src/hydrogen.cc#ne...
src/hydrogen.cc:612: HConstant* constant = HConstant::New(zone(), NULL, value);
NULL -> GetInvalidContext()

https://chromiumcodereview.appspot.com/21356002/diff/18001/src/hydrogen.cc#ne...
src/hydrogen.cc:1431: HLoadKeyed* load =
HLoadKeyed::cast(Add<HLoadKeyed>(external_elements,
Unnecessary HLoadKeyed::cast

https://chromiumcodereview.appspot.com/21356002/diff/18001/src/hydrogen.cc#ne...
src/hydrogen.cc:6381: arguments_object, undefined_receiver, zone());
Isn't the zone passed twice here?

https://chromiumcodereview.appspot.com/21356002/diff/18001/src/hydrogen.cc#ne...
src/hydrogen.cc:7469: mode, after);
Weird indentation.

[danno, 2013-07-31 14:49:21]

please take another look

https://codereview.chromium.org/21356002/diff/18001/src/hydrogen.cc
File src/hydrogen.cc (right):

https://codereview.chromium.org/21356002/diff/18001/src/hydrogen.cc#newcode612
src/hydrogen.cc:612: HConstant* constant = HConstant::New(zone(), NULL, value);
Cant do that here, since to get the invalid context you need to cal GetConstant
and that infinitely recurses. I'll add a comment.

On 2013/07/31 14:32:53, Toon Verwaest wrote:
> NULL -> GetInvalidContext()

https://codereview.chromium.org/21356002/diff/18001/src/hydrogen.cc#newcode1431
src/hydrogen.cc:1431: HLoadKeyed* load =
HLoadKeyed::cast(Add<HLoadKeyed>(external_elements,
On 2013/07/31 14:32:53, Toon Verwaest wrote:
> Unnecessary HLoadKeyed::cast

Done.

https://codereview.chromium.org/21356002/diff/18001/src/hydrogen.cc#newcode6381
src/hydrogen.cc:6381: arguments_object, undefined_receiver, zone());
On 2013/07/31 14:32:53, Toon Verwaest wrote:
> Isn't the zone passed twice here?

Done.

https://codereview.chromium.org/21356002/diff/18001/src/hydrogen.cc#newcode7469
src/hydrogen.cc:7469: mode, after);
On 2013/07/31 14:32:53, Toon Verwaest wrote:
> Weird indentation.

Done.

[Toon Verwaest, 2013-07-31 14:53:50]

lgtm

[danno, 2013-07-31 14:58:46]

Committed patchset #5 manually as r15990 (presubmit successful).