Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2755973002:
[type profile] Collect return types. (Closed)
Description[type profile] Collect return types.
Collect type information of return values.
Use *one* feedback slot per function for all its return
statements. For assignments, we currently use
several slots per function, because not all
assignments refer to the same variable.
Instead of the variable names, pass the
source location and print the function name.
Add an integration test for --type-profile that checks
for crashes.
Remove type feedback for assignments for now as it convolutes the output.
************ Function with 2 return statements ********
function testFunction(param, flag) {
// We want to test 2 different return positions in one function.
if (flag) {
var first_var = param;
return first_var;
}
var second_var = param;
return second_var;
}
testFunction({});
testFunction(123, true);
testFunction('hello');
testFunction(undefined);
*******************************************************
************* Sample Output ***************************
Function: testFunction
424: Object
374: number
424: string
424: undefined
*******************************************************
Missing work:
* Handle fall-off returns
* Collect types for parameters
* Remove duplicates from the list of collected types and use a common base class.
BUG=v8:5935
Review-Url: https://codereview.chromium.org/2755973002
Cr-Commit-Position: refs/heads/master@{#43956}
Committed: https://chromium.googlesource.com/v8/v8/+/de04df74129bba5539c6b887a2d2c81015fff866
Patch Set 1 #Patch Set 2 : Fix off by 2 error. #Patch Set 3 : Handle non variables. #Patch Set 4 : One slot per function. #Patch Set 5 : Output function name for return types. #Patch Set 6 : Clean up. #
Total comments: 18
Patch Set 7 : Address review comments. #Patch Set 8 : Use the source position. #
Total comments: 4
Patch Set 9 : Delete unnecessary checks. #Patch Set 10 : Rebase. #
Messages
Total messages: 92 (74 generated)
The CQ bit was checked by franzih@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Description was changed from ========== [type profile] Collect source position. In addition to the variable names, pass the source location. Collect types for return statements as well. BUG= ========== to ========== [type profile] Collect source position. In addition to the variable names, pass the source location. Collect types for return statements as well. BUG=v8:5935 ==========
franzih@chromium.org changed reviewers: + yangguo@chromium.org
Description was changed from ========== [type profile] Collect source position. In addition to the variable names, pass the source location. Collect types for return statements as well. BUG=v8:5935 ========== to ========== [type profile] Collect source position. In addition to the variable names, pass the source location. Collect types for return statements as well. Sample output: Position: 273, my_var1 Object number string number undefined string Object Position: 495, my_var MyClass Object BUG=v8:5935 ==========
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: v8_linux64_gyp_rel_ng on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_gyp_rel_ng/build...) v8_linux64_gyp_rel_ng_triggered on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_gyp_rel_ng_trigg...)
The CQ bit was checked by franzih@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was checked by franzih@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by franzih@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Description was changed from ========== [type profile] Collect source position. In addition to the variable names, pass the source location. Collect types for return statements as well. Sample output: Position: 273, my_var1 Object number string number undefined string Object Position: 495, my_var MyClass Object BUG=v8:5935 ========== to ========== [type profile] Collect return types. Collect type information of return values. Use one feedback slot per function for all its return statements. In addition to the variable names, pass the source location. Add an integration test for --type-profile that check for crashes (and changes). Sample output for two variables in the same function: Position: 393, second_var Object number string undefined Position: 343, first_var Object number string undefined Object Remove type feedback for assignments for now as it convolutes the output. BUG=v8:5935 ==========
Description was changed from ========== [type profile] Collect return types. Collect type information of return values. Use one feedback slot per function for all its return statements. In addition to the variable names, pass the source location. Add an integration test for --type-profile that check for crashes (and changes). Sample output for two variables in the same function: Position: 393, second_var Object number string undefined Position: 343, first_var Object number string undefined Object Remove type feedback for assignments for now as it convolutes the output. BUG=v8:5935 ========== to ========== [type profile] Collect return types. Collect type information of return values. Use one feedback slot per function for all its return statements. In addition to the variable names, pass the source location. Add an integration test for --type-profile that checks for crashes (and changes). Sample output for two variables in the same function: Position: 393, second_var Object number string undefined Position: 343, first_var Object number string undefined Object Remove type feedback for assignments for now as it convolutes the output. BUG=v8:5935 ==========
franzih@chromium.org changed reviewers: + ahaas@chromium.org
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: v8_win_nosnap_shared_rel_ng on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_win_nosnap_shared_rel_ng...) v8_win_nosnap_shared_rel_ng_triggered on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_win_nosnap_shared_rel_ng...)
The CQ bit was checked by franzih@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Patchset #5 (id:70001) has been deleted
The CQ bit was checked by franzih@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Description was changed from ========== [type profile] Collect return types. Collect type information of return values. Use one feedback slot per function for all its return statements. In addition to the variable names, pass the source location. Add an integration test for --type-profile that checks for crashes (and changes). Sample output for two variables in the same function: Position: 393, second_var Object number string undefined Position: 343, first_var Object number string undefined Object Remove type feedback for assignments for now as it convolutes the output. BUG=v8:5935 ========== to ========== [type profile] Collect return types. Collect type information of return values. Use one feedback slot per function for all its return statements. In addition to the variable names, pass the source location and print the function name. Add an integration test for --type-profile that checks for crashes (and changes). Sample output for two variables in the same function: Function: testFunction, position: 401, expression: second_var Object number Function: testFunction, position: 351, expression: first_var Object number string Remove type feedback for assignments for now as it convolutes the output. BUG=v8:5935 ==========
Description was changed from ========== [type profile] Collect return types. Collect type information of return values. Use one feedback slot per function for all its return statements. In addition to the variable names, pass the source location and print the function name. Add an integration test for --type-profile that checks for crashes (and changes). Sample output for two variables in the same function: Function: testFunction, position: 401, expression: second_var Object number Function: testFunction, position: 351, expression: first_var Object number string Remove type feedback for assignments for now as it convolutes the output. BUG=v8:5935 ========== to ========== [type profile] Collect return types. Collect type information of return values. Use one feedback slot per function for all its return statements. In addition to the variable names, pass the source location and print the function name. Add an integration test for --type-profile that checks for crashes. Sample output for two variables in the same function: Function: testFunction, position: 401, expression: second_var Object number Function: testFunction, position: 351, expression: first_var Object number string Remove type feedback for assignments for now as it convolutes the output. BUG=v8:5935 ==========
Description was changed from
==========
[type profile] Collect return types.
Collect type information of return values.
Use one feedback slot per function for all its return
statements.
In addition to the variable names, pass the
source location and print the function name.
Add an integration test for --type-profile that checks
for crashes.
Sample output for two variables in the same function:
Function: testFunction, position: 401, expression: second_var
Object
number
Function: testFunction, position: 351, expression: first_var
Object
number
string
Remove type feedback for assignments for now as it convolutes the output.
BUG=v8:5935
==========
to
==========
[type profile] Collect return types.
Collect type information of return values.
Use one feedback slot per function for all its return
statements.
In addition to the variable names, pass the
source location and print the function name.
Add an integration test for --type-profile that checks
for crashes.
Remove type feedback for assignments for now as it convolutes the output.
************ Function with 2 return statements ********
function testFunction(param, flag) {
// We want to test 2 different return positions in one function.
if (flag) {
var first_var = param;
return first_var;
}
var second_var = param;
return second_var;
}
testFunction({});
testFunction(123);
testFunction('hello', true);
testFunction(undefined);
*******************************************************
************* Sample Output ***************************
Function: testFunction, position: 351, expression: first_var
Object
number
string
Function: testFunction, position: 401, expression: second_var
Object
number
string
undefined
*******************************************************
BUG=v8:5935
==========
The CQ bit was checked by franzih@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Description was changed from
==========
[type profile] Collect return types.
Collect type information of return values.
Use one feedback slot per function for all its return
statements.
In addition to the variable names, pass the
source location and print the function name.
Add an integration test for --type-profile that checks
for crashes.
Remove type feedback for assignments for now as it convolutes the output.
************ Function with 2 return statements ********
function testFunction(param, flag) {
// We want to test 2 different return positions in one function.
if (flag) {
var first_var = param;
return first_var;
}
var second_var = param;
return second_var;
}
testFunction({});
testFunction(123);
testFunction('hello', true);
testFunction(undefined);
*******************************************************
************* Sample Output ***************************
Function: testFunction, position: 351, expression: first_var
Object
number
string
Function: testFunction, position: 401, expression: second_var
Object
number
string
undefined
*******************************************************
BUG=v8:5935
==========
to
==========
[type profile] Collect return types.
Collect type information of return values.
Use *one* feedback slot per function for all its return
statements. For assignments, we currently use
several slots per function, because not all
assignments refer to the same variable.
In addition to the variable names, pass the
source location and print the function name.
Add an integration test for --type-profile that checks
for crashes.
Remove type feedback for assignments for now as it convolutes the output.
************ Function with 2 return statements ********
function testFunction(param, flag) {
// We want to test 2 different return positions in one function.
if (flag) {
var first_var = param;
return first_var;
}
var second_var = param;
return second_var;
}
testFunction({});
testFunction(123);
testFunction('hello', true);
testFunction(undefined);
*******************************************************
************* Sample Output ***************************
Function: testFunction, position: 351, expression: first_var
Object
number
string
Function: testFunction, position: 401, expression: second_var
Object
number
string
undefined
*******************************************************
BUG=v8:5935
==========
franzih@chromium.org changed reviewers: + mstarzinger@chromium.org - ahaas@chromium.org, yangguo@chromium.org
Hi Michael, Can you have a look? Type profiling is still WIP. As discussed offline, I'll change the implementation to use one slot for all type information (return value, parameters, variable declarations) in a follow up CL. Thanks, Franzi
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
franzih@chromium.org changed reviewers: + yangguo@chromium.org
Looking got, just some minor comments. https://codereview.chromium.org/2755973002/diff/110001/src/ast/ast-numbering.cc File src/ast/ast-numbering.cc (right): https://codereview.chromium.org/2755973002/diff/110001/src/ast/ast-numbering.... src/ast/ast-numbering.cc:678: type_profile_for_return_value_ = nit: Please move this down to after the global compiler and into the language mode scope (i.e. to line 708 ideally). https://codereview.chromium.org/2755973002/diff/110001/src/ast/ast.h File src/ast/ast.h (right): https://codereview.chromium.org/2755973002/diff/110001/src/ast/ast.h#newcode898 src/ast/ast.h:898: class ReturnStatement final : public JumpStatement { High level remark: Note that the implicit return value when one falls off the function end is not captured by the AST. That means e.g. function not containing any explicit "return" statement does not contain any {ReturnStatement} AST node, the fall-off return instruction is added individually by the various compilers. Also, any "yield" point within generator functions is not covered by this either. But I assume that is actually working as intended. https://codereview.chromium.org/2755973002/diff/110001/src/interpreter/byteco... File src/interpreter/bytecode-generator.cc (right): https://codereview.chromium.org/2755973002/diff/110001/src/interpreter/byteco... src/interpreter/bytecode-generator.cc:2399: if (false && expr->HasTypeProfileSlot()) { nit: Looks like a left-over. If it is intentional then please leave a TODO here explaining why and how long this is intended to be dead code. https://codereview.chromium.org/2755973002/diff/110001/src/runtime/runtime-ob... File src/runtime/runtime-object.cc (right): https://codereview.chromium.org/2755973002/diff/110001/src/runtime/runtime-ob... src/runtime/runtime-object.cc:705: Object* function_name = vector->shared_function_info()->name(); This must be handlified, otherwise the handlified function calls between here and the use of the {function_name} variable below could trigger a GC and leave this raw pointer in a stale state. As a general rule of thumb: Either handlify the entire function and leave the entire function to raw pointers. Mixing raw pointers and handles is dangerous.
https://codereview.chromium.org/2755973002/diff/110001/src/runtime/runtime-ob... File src/runtime/runtime-object.cc (right): https://codereview.chromium.org/2755973002/diff/110001/src/runtime/runtime-ob... src/runtime/runtime-object.cc:705: Object* function_name = vector->shared_function_info()->name(); On 2017/03/20 09:29:26, Michael Starzinger wrote: > This must be handlified, otherwise the handlified function calls between here > and the use of the {function_name} variable below could trigger a GC and leave > this raw pointer in a stale state. As a general rule of thumb: Either handlify > the entire function and leave the entire function to raw pointers. Mixing raw > pointers and handles is dangerous. s/function and leave/function or leave/
ahaas@chromium.org changed reviewers: + ahaas@chromium.org
https://codereview.chromium.org/2755973002/diff/110001/src/compiler/bytecode-... File src/compiler/bytecode-graph-builder.h (right): https://codereview.chromium.org/2755973002/diff/110001/src/compiler/bytecode-... src/compiler/bytecode-graph-builder.h:87: Node* NewNode(const Operator* op, Node* n1, Node* n2, Node* n3, Node* n4, You could be super fancy and do this with variadic templates. Instead of introducing a new NewNode function, you could replace all NewNode functions by template<typename... Nodes> Node* NewNode(const Operator* op, Nodes... n) { Node* buffer[] = {n...}; return MakeNode(op, arraysize(buffer), buffer, false); }
https://codereview.chromium.org/2755973002/diff/110001/src/compiler/bytecode-... File src/compiler/bytecode-graph-builder.h (right): https://codereview.chromium.org/2755973002/diff/110001/src/compiler/bytecode-... src/compiler/bytecode-graph-builder.h:87: Node* NewNode(const Operator* op, Node* n1, Node* n2, Node* n3, Node* n4, On 2017/03/20 at 09:34:43, ahaas wrote: > You could be super fancy and do this with variadic templates. Instead of introducing a new NewNode function, you could replace all NewNode functions by > template<typename... Nodes> > Node* NewNode(const Operator* op, Nodes... n) { > Node* buffer[] = {n...}; > return MakeNode(op, arraysize(buffer), buffer, false); > } better use template<typename... Nodes> Node* NewNode(const Operator* op, Node* n0, Nodes... n) { Node* buffer[] = {n0, n...}; return MakeNode(op, arraysize(buffer), buffer, false); } because otherwise there could be a signature overlap with Node* NewNode(const Operator* op, bool incomplete = false)
rmcilroy@chromium.org changed reviewers: + rmcilroy@chromium.org
https://codereview.chromium.org/2755973002/diff/110001/src/interpreter/byteco... File src/interpreter/bytecode-generator.cc (right): https://codereview.chromium.org/2755973002/diff/110001/src/interpreter/byteco... src/interpreter/bytecode-generator.cc:1102: .StoreAccumulatorInRegister(position); Any reason you need to store position as a register? You could just make it a kUImm or kImm operand type and store it directly in the bytecode. https://codereview.chromium.org/2755973002/diff/110001/src/interpreter/byteco... src/interpreter/bytecode-generator.cc:1116: } As I remember, the plan was to remove the need for the name operand once the source position was added. Could you do that here to avoid needing additional logic to get the name for return statements?
https://codereview.chromium.org/2755973002/diff/110001/src/ast/ast.h File src/ast/ast.h (right): https://codereview.chromium.org/2755973002/diff/110001/src/ast/ast.h#newcode898 src/ast/ast.h:898: class ReturnStatement final : public JumpStatement { On 2017/03/20 09:29:26, Michael Starzinger wrote: > High level remark: Note that the implicit return value when one falls off the > function end is not captured by the AST. That means e.g. function not containing > any explicit "return" statement does not contain any {ReturnStatement} AST node, > the fall-off return instruction is added individually by the various compilers. > > Also, any "yield" point within generator functions is not covered by this > either. But I assume that is actually working as intended. So if the function only has a fall-off return, the observed type is "undefined" by definition, right?
On 2017/03/20 10:07:24, Yang wrote: > https://codereview.chromium.org/2755973002/diff/110001/src/ast/ast.h > File src/ast/ast.h (right): > > https://codereview.chromium.org/2755973002/diff/110001/src/ast/ast.h#newcode898 > src/ast/ast.h:898: class ReturnStatement final : public JumpStatement { > On 2017/03/20 09:29:26, Michael Starzinger wrote: > > High level remark: Note that the implicit return value when one falls off the > > function end is not captured by the AST. That means e.g. function not > containing > > any explicit "return" statement does not contain any {ReturnStatement} AST > node, > > the fall-off return instruction is added individually by the various > compilers. > > > > Also, any "yield" point within generator functions is not covered by this > > either. But I assume that is actually working as intended. > > So if the function only has a fall-off return, the observed type is "undefined" > by definition, right? Correct. But that might "mix" with other explicit return values, not sure if this will affect how the feedback is presented to the user further down the line. I am thinking about a function like the following: function f() { if (Math.random() < 0.5) { return "I sometimes return a string"; } // ... but half of the time I just fall off the end. }
On 2017/03/20 10:15:59, Michael Starzinger wrote: > On 2017/03/20 10:07:24, Yang wrote: > > https://codereview.chromium.org/2755973002/diff/110001/src/ast/ast.h > > File src/ast/ast.h (right): > > > > > https://codereview.chromium.org/2755973002/diff/110001/src/ast/ast.h#newcode898 > > src/ast/ast.h:898: class ReturnStatement final : public JumpStatement { > > On 2017/03/20 09:29:26, Michael Starzinger wrote: > > > High level remark: Note that the implicit return value when one falls off > the > > > function end is not captured by the AST. That means e.g. function not > > containing > > > any explicit "return" statement does not contain any {ReturnStatement} AST > > node, > > > the fall-off return instruction is added individually by the various > > compilers. > > > > > > Also, any "yield" point within generator functions is not covered by this > > > either. But I assume that is actually working as intended. > > > > So if the function only has a fall-off return, the observed type is > "undefined" > > by definition, right? > > Correct. But that might "mix" with other explicit return values, not sure if > this will affect how the feedback is presented to the user further down the > line. I am thinking about a function like the following: > > function f() { > if (Math.random() < 0.5) { > return "I sometimes return a string"; > } > // ... but half of the time I just fall off the end. > } Right. But in that case we do have an explicit return statement, so we allocated a type profile slot for returns. The fall-off return can then record the undefined type to that type profile slot.
On 2017/03/20 10:24:42, Yang wrote: > On 2017/03/20 10:15:59, Michael Starzinger wrote: > > On 2017/03/20 10:07:24, Yang wrote: > > > https://codereview.chromium.org/2755973002/diff/110001/src/ast/ast.h > > > File src/ast/ast.h (right): > > > > > > > > > https://codereview.chromium.org/2755973002/diff/110001/src/ast/ast.h#newcode898 > > > src/ast/ast.h:898: class ReturnStatement final : public JumpStatement { > > > On 2017/03/20 09:29:26, Michael Starzinger wrote: > > > > High level remark: Note that the implicit return value when one falls off > > the > > > > function end is not captured by the AST. That means e.g. function not > > > containing > > > > any explicit "return" statement does not contain any {ReturnStatement} AST > > > node, > > > > the fall-off return instruction is added individually by the various > > > compilers. > > > > > > > > Also, any "yield" point within generator functions is not covered by this > > > > either. But I assume that is actually working as intended. > > > > > > So if the function only has a fall-off return, the observed type is > > "undefined" > > > by definition, right? > > > > Correct. But that might "mix" with other explicit return values, not sure if > > this will affect how the feedback is presented to the user further down the > > line. I am thinking about a function like the following: > > > > function f() { > > if (Math.random() < 0.5) { > > return "I sometimes return a string"; > > } > > // ... but half of the time I just fall off the end. > > } > > Right. But in that case we do have an explicit return statement, so we allocated > a type profile slot for returns. The fall-off return can then record the > undefined type to that type profile slot. Of course the easier thing to do is to always allocate a type profile slot for return, independent from whether the AST has a ReturnStatement.
On 2017/03/20 10:25:21, Yang wrote: > On 2017/03/20 10:24:42, Yang wrote: > > On 2017/03/20 10:15:59, Michael Starzinger wrote: > > > On 2017/03/20 10:07:24, Yang wrote: > > > > https://codereview.chromium.org/2755973002/diff/110001/src/ast/ast.h > > > > File src/ast/ast.h (right): > > > > > > > > > > > > > > https://codereview.chromium.org/2755973002/diff/110001/src/ast/ast.h#newcode898 > > > > src/ast/ast.h:898: class ReturnStatement final : public JumpStatement { > > > > On 2017/03/20 09:29:26, Michael Starzinger wrote: > > > > > High level remark: Note that the implicit return value when one falls > off > > > the > > > > > function end is not captured by the AST. That means e.g. function not > > > > containing > > > > > any explicit "return" statement does not contain any {ReturnStatement} > AST > > > > node, > > > > > the fall-off return instruction is added individually by the various > > > > compilers. > > > > > > > > > > Also, any "yield" point within generator functions is not covered by > this > > > > > either. But I assume that is actually working as intended. > > > > > > > > So if the function only has a fall-off return, the observed type is > > > "undefined" > > > > by definition, right? > > > > > > Correct. But that might "mix" with other explicit return values, not sure if > > > this will affect how the feedback is presented to the user further down the > > > line. I am thinking about a function like the following: > > > > > > function f() { > > > if (Math.random() < 0.5) { > > > return "I sometimes return a string"; > > > } > > > // ... but half of the time I just fall off the end. > > > } > > > > Right. But in that case we do have an explicit return statement, so we > allocated > > a type profile slot for returns. The fall-off return can then record the > > undefined type to that type profile slot. > > Of course the easier thing to do is to always allocate a type profile slot for > return, independent from whether the AST has a ReturnStatement. You would also need to do the type recording in GenerateBytecode under "builder()->RequiresImplicitReturn()" I guess?
On 2017/03/20 10:24:42, Yang wrote: > On 2017/03/20 10:15:59, Michael Starzinger wrote: > > On 2017/03/20 10:07:24, Yang wrote: > > > https://codereview.chromium.org/2755973002/diff/110001/src/ast/ast.h > > > File src/ast/ast.h (right): > > > > > > > > > https://codereview.chromium.org/2755973002/diff/110001/src/ast/ast.h#newcode898 > > > src/ast/ast.h:898: class ReturnStatement final : public JumpStatement { > > > On 2017/03/20 09:29:26, Michael Starzinger wrote: > > > > High level remark: Note that the implicit return value when one falls off > > the > > > > function end is not captured by the AST. That means e.g. function not > > > containing > > > > any explicit "return" statement does not contain any {ReturnStatement} AST > > > node, > > > > the fall-off return instruction is added individually by the various > > > compilers. > > > > > > > > Also, any "yield" point within generator functions is not covered by this > > > > either. But I assume that is actually working as intended. > > > > > > So if the function only has a fall-off return, the observed type is > > "undefined" > > > by definition, right? > > > > Correct. But that might "mix" with other explicit return values, not sure if > > this will affect how the feedback is presented to the user further down the > > line. I am thinking about a function like the following: > > > > function f() { > > if (Math.random() < 0.5) { > > return "I sometimes return a string"; > > } > > // ... but half of the time I just fall off the end. > > } > > Right. But in that case we do have an explicit return statement, so we allocated > a type profile slot for returns. The fall-off return can then record the > undefined type to that type profile slot. With the CL in its current form, this recording of the fall-off return will not happen. That is exactly the point I am trying to make. We need some way to thread through the necessary feedback slot to {BytecodeGenerator::GenerateBytecode} where we don't have access to a specific {Return} AST node holding that information. See here: https://cs.chromium.org/chromium/src/v8/src/interpreter/bytecode-generator.cc...
The CQ bit was checked by franzih@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was checked by franzih@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: v8_linux_verify_csa_rel_ng on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux_verify_csa_rel_ng/...) v8_linux_verify_csa_rel_ng_triggered on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux_verify_csa_rel_ng_...)
Patchset #8 (id:150001) has been deleted
Patchset #7 (id:130001) has been deleted
The CQ bit was checked by franzih@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Description was changed from
==========
[type profile] Collect return types.
Collect type information of return values.
Use *one* feedback slot per function for all its return
statements. For assignments, we currently use
several slots per function, because not all
assignments refer to the same variable.
In addition to the variable names, pass the
source location and print the function name.
Add an integration test for --type-profile that checks
for crashes.
Remove type feedback for assignments for now as it convolutes the output.
************ Function with 2 return statements ********
function testFunction(param, flag) {
// We want to test 2 different return positions in one function.
if (flag) {
var first_var = param;
return first_var;
}
var second_var = param;
return second_var;
}
testFunction({});
testFunction(123);
testFunction('hello', true);
testFunction(undefined);
*******************************************************
************* Sample Output ***************************
Function: testFunction, position: 351, expression: first_var
Object
number
string
Function: testFunction, position: 401, expression: second_var
Object
number
string
undefined
*******************************************************
BUG=v8:5935
==========
to
==========
[type profile] Collect return types.
Collect type information of return values.
Use *one* feedback slot per function for all its return
statements. For assignments, we currently use
several slots per function, because not all
assignments refer to the same variable.
Instead of the variable names, pass the
source location and print the function name.
Add an integration test for --type-profile that checks
for crashes.
Remove type feedback for assignments for now as it convolutes the output.
************ Function with 2 return statements ********
function testFunction(param, flag) {
// We want to test 2 different return positions in one function.
if (flag) {
var first_var = param;
return first_var;
}
var second_var = param;
return second_var;
}
testFunction({});
testFunction(123);
testFunction('hello', true);
testFunction(undefined);
*******************************************************
************* Sample Output ***************************
Function: testFunction
Object
number
string
Function: testFunction
number
string
undefined
*******************************************************
BUG=v8:5935
==========
Description was changed from
==========
[type profile] Collect return types.
Collect type information of return values.
Use *one* feedback slot per function for all its return
statements. For assignments, we currently use
several slots per function, because not all
assignments refer to the same variable.
Instead of the variable names, pass the
source location and print the function name.
Add an integration test for --type-profile that checks
for crashes.
Remove type feedback for assignments for now as it convolutes the output.
************ Function with 2 return statements ********
function testFunction(param, flag) {
// We want to test 2 different return positions in one function.
if (flag) {
var first_var = param;
return first_var;
}
var second_var = param;
return second_var;
}
testFunction({});
testFunction(123);
testFunction('hello', true);
testFunction(undefined);
*******************************************************
************* Sample Output ***************************
Function: testFunction
Object
number
string
Function: testFunction
number
string
undefined
*******************************************************
BUG=v8:5935
==========
to
==========
[type profile] Collect return types.
Collect type information of return values.
Use *one* feedback slot per function for all its return
statements. For assignments, we currently use
several slots per function, because not all
assignments refer to the same variable.
Instead of the variable names, pass the
source location and print the function name.
Add an integration test for --type-profile that checks
for crashes.
Remove type feedback for assignments for now as it convolutes the output.
************ Function with 2 return statements ********
function testFunction(param, flag) {
// We want to test 2 different return positions in one function.
if (flag) {
var first_var = param;
return first_var;
}
var second_var = param;
return second_var;
}
testFunction({});
testFunction(123);
testFunction('hello', true);
testFunction(undefined);
*******************************************************
************* Sample Output ***************************
Function: testFunction
Object
number
string
Function: testFunction
number
string
undefined
*******************************************************
Missing work:
* Handle fall-off returns
* Collect types for parameters
BUG=v8:5935
==========
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
Description was changed from
==========
[type profile] Collect return types.
Collect type information of return values.
Use *one* feedback slot per function for all its return
statements. For assignments, we currently use
several slots per function, because not all
assignments refer to the same variable.
Instead of the variable names, pass the
source location and print the function name.
Add an integration test for --type-profile that checks
for crashes.
Remove type feedback for assignments for now as it convolutes the output.
************ Function with 2 return statements ********
function testFunction(param, flag) {
// We want to test 2 different return positions in one function.
if (flag) {
var first_var = param;
return first_var;
}
var second_var = param;
return second_var;
}
testFunction({});
testFunction(123);
testFunction('hello', true);
testFunction(undefined);
*******************************************************
************* Sample Output ***************************
Function: testFunction
Object
number
string
Function: testFunction
number
string
undefined
*******************************************************
Missing work:
* Handle fall-off returns
* Collect types for parameters
BUG=v8:5935
==========
to
==========
[type profile] Collect return types.
Collect type information of return values.
Use *one* feedback slot per function for all its return
statements. For assignments, we currently use
several slots per function, because not all
assignments refer to the same variable.
Instead of the variable names, pass the
source location and print the function name.
Add an integration test for --type-profile that checks
for crashes.
Remove type feedback for assignments for now as it convolutes the output.
************ Function with 2 return statements ********
function testFunction(param, flag) {
// We want to test 2 different return positions in one function.
if (flag) {
var first_var = param;
return first_var;
}
var second_var = param;
return second_var;
}
testFunction({});
testFunction(123);
testFunction('hello', true);
testFunction(undefined);
*******************************************************
************* Sample Output ***************************
Function: testFunction
Object
number
string
Function: testFunction
number
string
undefined
*******************************************************
Missing work:
* Handle fall-off returns
* Collect types for parameters
* Remove duplicates from the list of collected types and use a common base
class.
BUG=v8:5935
==========
The CQ bit was checked by franzih@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Description was changed from
==========
[type profile] Collect return types.
Collect type information of return values.
Use *one* feedback slot per function for all its return
statements. For assignments, we currently use
several slots per function, because not all
assignments refer to the same variable.
Instead of the variable names, pass the
source location and print the function name.
Add an integration test for --type-profile that checks
for crashes.
Remove type feedback for assignments for now as it convolutes the output.
************ Function with 2 return statements ********
function testFunction(param, flag) {
// We want to test 2 different return positions in one function.
if (flag) {
var first_var = param;
return first_var;
}
var second_var = param;
return second_var;
}
testFunction({});
testFunction(123);
testFunction('hello', true);
testFunction(undefined);
*******************************************************
************* Sample Output ***************************
Function: testFunction
Object
number
string
Function: testFunction
number
string
undefined
*******************************************************
Missing work:
* Handle fall-off returns
* Collect types for parameters
* Remove duplicates from the list of collected types and use a common base
class.
BUG=v8:5935
==========
to
==========
[type profile] Collect return types.
Collect type information of return values.
Use *one* feedback slot per function for all its return
statements. For assignments, we currently use
several slots per function, because not all
assignments refer to the same variable.
Instead of the variable names, pass the
source location and print the function name.
Add an integration test for --type-profile that checks
for crashes.
Remove type feedback for assignments for now as it convolutes the output.
************ Function with 2 return statements ********
function testFunction(param, flag) {
// We want to test 2 different return positions in one function.
if (flag) {
var first_var = param;
return first_var;
}
var second_var = param;
return second_var;
}
testFunction({});
testFunction(123);
testFunction('hello', true);
testFunction(undefined);
*******************************************************
************* Sample Output ***************************
Function: testFunction
424: Object
374: number
424: string
424: number
424: undefined
*******************************************************
Missing work:
* Handle fall-off returns
* Collect types for parameters
* Remove duplicates from the list of collected types and use a common base
class.
BUG=v8:5935
==========
Description was changed from
==========
[type profile] Collect return types.
Collect type information of return values.
Use *one* feedback slot per function for all its return
statements. For assignments, we currently use
several slots per function, because not all
assignments refer to the same variable.
Instead of the variable names, pass the
source location and print the function name.
Add an integration test for --type-profile that checks
for crashes.
Remove type feedback for assignments for now as it convolutes the output.
************ Function with 2 return statements ********
function testFunction(param, flag) {
// We want to test 2 different return positions in one function.
if (flag) {
var first_var = param;
return first_var;
}
var second_var = param;
return second_var;
}
testFunction({});
testFunction(123);
testFunction('hello', true);
testFunction(undefined);
*******************************************************
************* Sample Output ***************************
Function: testFunction
424: Object
374: number
424: string
424: number
424: undefined
*******************************************************
Missing work:
* Handle fall-off returns
* Collect types for parameters
* Remove duplicates from the list of collected types and use a common base
class.
BUG=v8:5935
==========
to
==========
[type profile] Collect return types.
Collect type information of return values.
Use *one* feedback slot per function for all its return
statements. For assignments, we currently use
several slots per function, because not all
assignments refer to the same variable.
Instead of the variable names, pass the
source location and print the function name.
Add an integration test for --type-profile that checks
for crashes.
Remove type feedback for assignments for now as it convolutes the output.
************ Function with 2 return statements ********
function testFunction(param, flag) {
// We want to test 2 different return positions in one function.
if (flag) {
var first_var = param;
return first_var;
}
var second_var = param;
return second_var;
}
testFunction({});
testFunction(123, true);
testFunction('hello');
testFunction(undefined);
*******************************************************
************* Sample Output ***************************
Function: testFunction
424: Object
374: number
424: string
424: undefined
*******************************************************
Missing work:
* Handle fall-off returns
* Collect types for parameters
* Remove duplicates from the list of collected types and use a common base
class.
BUG=v8:5935
==========
Hi Michi, hi Ross, I addressed your comments and removed all of the extra logic for passing the variable name around. I want to address these points in follow up CLs: * Handle fall-off returns * Collect types for parameters * Remove duplicates from the list of collected types and use a common base class Please take another look. Thanks, Franzi https://codereview.chromium.org/2755973002/diff/110001/src/ast/ast-numbering.cc File src/ast/ast-numbering.cc (right): https://codereview.chromium.org/2755973002/diff/110001/src/ast/ast-numbering.... src/ast/ast-numbering.cc:678: type_profile_for_return_value_ = On 2017/03/20 09:29:26, Michael Starzinger wrote: > nit: Please move this down to after the global compiler and into the language > mode scope (i.e. to line 708 ideally). Done. https://codereview.chromium.org/2755973002/diff/110001/src/ast/ast.h File src/ast/ast.h (right): https://codereview.chromium.org/2755973002/diff/110001/src/ast/ast.h#newcode898 src/ast/ast.h:898: class ReturnStatement final : public JumpStatement { On 2017/03/20 10:07:23, Yang wrote: > On 2017/03/20 09:29:26, Michael Starzinger wrote: > > High level remark: Note that the implicit return value when one falls off the > > function end is not captured by the AST. That means e.g. function not > containing > > any explicit "return" statement does not contain any {ReturnStatement} AST > node, > > the fall-off return instruction is added individually by the various > compilers. > > > > Also, any "yield" point within generator functions is not covered by this > > either. But I assume that is actually working as intended. > > So if the function only has a fall-off return, the observed type is "undefined" > by definition, right? Oh, I didn't think of fall-off returns. If they happen, then "undefined" should definitely be added to the types. Is it OK to do that in a follow up CL? In the current form, for a fall-off return, "VisitReturnStatement" is not called, so the CollectTypeProfile bytecode is not emitted, and it's only that "undefined" is missing in the list of types, not that anything crashes, right? https://codereview.chromium.org/2755973002/diff/110001/src/compiler/bytecode-... File src/compiler/bytecode-graph-builder.h (right): https://codereview.chromium.org/2755973002/diff/110001/src/compiler/bytecode-... src/compiler/bytecode-graph-builder.h:87: Node* NewNode(const Operator* op, Node* n1, Node* n2, Node* n3, Node* n4, On 2017/03/20 09:53:36, ahaas wrote: > On 2017/03/20 at 09:34:43, ahaas wrote: > > You could be super fancy and do this with variadic templates. Instead of > introducing a new NewNode function, you could replace all NewNode functions by > > template<typename... Nodes> > > Node* NewNode(const Operator* op, Nodes... n) { > > Node* buffer[] = {n...}; > > return MakeNode(op, arraysize(buffer), buffer, false); > > } > > better use > > template<typename... Nodes> > Node* NewNode(const Operator* op, Node* n0, Nodes... n) { > Node* buffer[] = {n0, n...}; > return MakeNode(op, arraysize(buffer), buffer, false); > } > > because otherwise there could be a signature overlap with > Node* NewNode(const Operator* op, bool incomplete = false) No chance to be super fancy :( Don't need NewNode() with 5 nodes anymore. https://codereview.chromium.org/2755973002/diff/110001/src/interpreter/byteco... File src/interpreter/bytecode-generator.cc (right): https://codereview.chromium.org/2755973002/diff/110001/src/interpreter/byteco... src/interpreter/bytecode-generator.cc:1102: .StoreAccumulatorInRegister(position); On 2017/03/20 10:02:46, rmcilroy wrote: > Any reason you need to store position as a register? You could just make it a > kUImm or kImm operand type and store it directly in the bytecode. Done. Using kImm because for the global(?) return the position is -1. https://codereview.chromium.org/2755973002/diff/110001/src/interpreter/byteco... src/interpreter/bytecode-generator.cc:1116: } On 2017/03/20 10:02:46, rmcilroy wrote: > As I remember, the plan was to remove the need for the name operand once the > source position was added. Could you do that here to avoid needing additional > logic to get the name for return statements? Done. https://codereview.chromium.org/2755973002/diff/110001/src/interpreter/byteco... src/interpreter/bytecode-generator.cc:2399: if (false && expr->HasTypeProfileSlot()) { On 2017/03/20 09:29:26, Michael Starzinger wrote: > nit: Looks like a left-over. If it is intentional then please leave a TODO here > explaining why and how long this is intended to be dead code. Done. https://codereview.chromium.org/2755973002/diff/110001/src/runtime/runtime-ob... File src/runtime/runtime-object.cc (right): https://codereview.chromium.org/2755973002/diff/110001/src/runtime/runtime-ob... src/runtime/runtime-object.cc:705: Object* function_name = vector->shared_function_info()->name(); On 2017/03/20 09:31:48, Michael Starzinger wrote: > On 2017/03/20 09:29:26, Michael Starzinger wrote: > > This must be handlified, otherwise the handlified function calls between here > > and the use of the {function_name} variable below could trigger a GC and leave > > this raw pointer in a stale state. As a general rule of thumb: Either handlify > > the entire function and leave the entire function to raw pointers. Mixing raw > > pointers and handles is dangerous. > > s/function and leave/function or leave/ Done.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
LGTM from my side. https://codereview.chromium.org/2755973002/diff/110001/src/ast/ast.h File src/ast/ast.h (right): https://codereview.chromium.org/2755973002/diff/110001/src/ast/ast.h#newcode898 src/ast/ast.h:898: class ReturnStatement final : public JumpStatement { On 2017/03/20 15:38:39, Franzi wrote: > On 2017/03/20 10:07:23, Yang wrote: > > On 2017/03/20 09:29:26, Michael Starzinger wrote: > > > High level remark: Note that the implicit return value when one falls off > the > > > function end is not captured by the AST. That means e.g. function not > > containing > > > any explicit "return" statement does not contain any {ReturnStatement} AST > > node, > > > the fall-off return instruction is added individually by the various > > compilers. > > > > > > Also, any "yield" point within generator functions is not covered by this > > > either. But I assume that is actually working as intended. > > > > So if the function only has a fall-off return, the observed type is > "undefined" > > by definition, right? > > Oh, I didn't think of fall-off returns. If they happen, then "undefined" should > definitely be added to the types. > > Is it OK to do that in a follow up CL? In the current form, for a fall-off > return, "VisitReturnStatement" is not called, so the CollectTypeProfile bytecode > is not emitted, and it's only that "undefined" is missing in the list of types, > not that anything crashes, right? Acknowledged. Totally fine with this being done later in a separate CL. Yes, should only cause "missing feedback", no crashes or anything scary like that. https://codereview.chromium.org/2755973002/diff/190001/src/feedback-vector.cc File src/feedback-vector.cc (right): https://codereview.chromium.org/2755973002/diff/190001/src/feedback-vector.cc... src/feedback-vector.cc:1086: if (!maybe_entry->IsTuple2()) { The Handle<Tuple2>::cast already covers this. If you want to be explicit (and/or want the check in release mode as well), then I would go with ... CHECK(maybe_entry->IsTuple2()); https://codereview.chromium.org/2755973002/diff/190001/src/runtime/runtime-ob... File src/runtime/runtime-object.cc (right): https://codereview.chromium.org/2755973002/diff/190001/src/runtime/runtime-ob... src/runtime/runtime-object.cc:704: Handle<Object> function_name = nit: The {function_name} variable seems to be unused now. Can we drop it?
Intepreter/ LGTM, thanks.
The CQ bit was checked by franzih@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: v8_linux64_asan_rel_ng on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_asan_rel_ng/buil...) v8_linux64_rel_ng on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_rel_ng/builds/22920) v8_linux64_verify_csa_rel_ng on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_verify_csa_rel_n...) v8_linux_arm_rel_ng on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux_arm_rel_ng/builds/...) v8_linux_gcc_compile_rel on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux_gcc_compile_rel/bu...) v8_linux_mips64el_compile_rel on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux_mips64el_compile_r...) v8_linux_verify_csa_rel_ng on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux_verify_csa_rel_ng/...)
The CQ bit was checked by franzih@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: v8_linux64_asan_rel_ng on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_asan_rel_ng/buil...) v8_linux64_asan_rel_ng_triggered on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_asan_rel_ng_trig...) v8_linux64_gyp_rel_ng on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_gyp_rel_ng/build...) v8_linux64_gyp_rel_ng_triggered on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_gyp_rel_ng_trigg...) v8_linux_arm_rel_ng on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux_arm_rel_ng/builds/...) v8_linux_chromium_gn_rel on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux_chromium_gn_rel/bu...) v8_linux_dbg_ng on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux_dbg_ng/builds/22947) v8_linux_nodcheck_rel_ng on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux_nodcheck_rel_ng/bu...) v8_linux_rel_ng on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux_rel_ng/builds/22838) v8_linux_verify_csa_rel_ng on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux_verify_csa_rel_ng/...)
The CQ bit was checked by franzih@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
https://codereview.chromium.org/2755973002/diff/190001/src/feedback-vector.cc File src/feedback-vector.cc (right): https://codereview.chromium.org/2755973002/diff/190001/src/feedback-vector.cc... src/feedback-vector.cc:1086: if (!maybe_entry->IsTuple2()) { On 2017/03/20 17:12:32, Michael Starzinger wrote: > The Handle<Tuple2>::cast already covers this. If you want to be explicit (and/or > want the check in release mode as well), then I would go with ... > > CHECK(maybe_entry->IsTuple2()); I have no idea what I was thinking 😂. Keeping a DCHECK to be explicit. https://codereview.chromium.org/2755973002/diff/190001/src/runtime/runtime-ob... File src/runtime/runtime-object.cc (right): https://codereview.chromium.org/2755973002/diff/190001/src/runtime/runtime-ob... src/runtime/runtime-object.cc:704: Handle<Object> function_name = On 2017/03/20 17:12:32, Michael Starzinger wrote: > nit: The {function_name} variable seems to be unused now. Can we drop it? Seems like I came up with my own pattern instead of DCHECKs. Yikes. Deleted. Double-checking the rest for this wonderful pattern.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by franzih@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from mstarzinger@chromium.org, rmcilroy@chromium.org Link to the patchset: https://codereview.chromium.org/2755973002/#ps230001 (title: "Rebase.")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 230001, "attempt_start_ts": 1490039377460840,
"parent_rev": "99743ad460ea5b9795ba9d70a074e75d7362a3d1", "commit_rev":
"de04df74129bba5539c6b887a2d2c81015fff866"}
Message was sent while issue was closed.
Description was changed from
==========
[type profile] Collect return types.
Collect type information of return values.
Use *one* feedback slot per function for all its return
statements. For assignments, we currently use
several slots per function, because not all
assignments refer to the same variable.
Instead of the variable names, pass the
source location and print the function name.
Add an integration test for --type-profile that checks
for crashes.
Remove type feedback for assignments for now as it convolutes the output.
************ Function with 2 return statements ********
function testFunction(param, flag) {
// We want to test 2 different return positions in one function.
if (flag) {
var first_var = param;
return first_var;
}
var second_var = param;
return second_var;
}
testFunction({});
testFunction(123, true);
testFunction('hello');
testFunction(undefined);
*******************************************************
************* Sample Output ***************************
Function: testFunction
424: Object
374: number
424: string
424: undefined
*******************************************************
Missing work:
* Handle fall-off returns
* Collect types for parameters
* Remove duplicates from the list of collected types and use a common base
class.
BUG=v8:5935
==========
to
==========
[type profile] Collect return types.
Collect type information of return values.
Use *one* feedback slot per function for all its return
statements. For assignments, we currently use
several slots per function, because not all
assignments refer to the same variable.
Instead of the variable names, pass the
source location and print the function name.
Add an integration test for --type-profile that checks
for crashes.
Remove type feedback for assignments for now as it convolutes the output.
************ Function with 2 return statements ********
function testFunction(param, flag) {
// We want to test 2 different return positions in one function.
if (flag) {
var first_var = param;
return first_var;
}
var second_var = param;
return second_var;
}
testFunction({});
testFunction(123, true);
testFunction('hello');
testFunction(undefined);
*******************************************************
************* Sample Output ***************************
Function: testFunction
424: Object
374: number
424: string
424: undefined
*******************************************************
Missing work:
* Handle fall-off returns
* Collect types for parameters
* Remove duplicates from the list of collected types and use a common base
class.
BUG=v8:5935
Review-Url: https://codereview.chromium.org/2755973002
Cr-Commit-Position: refs/heads/master@{#43956}
Committed:
https://chromium.googlesource.com/v8/v8/+/de04df74129bba5539c6b887a2d2c81015f...
==========
Message was sent while issue was closed.
Committed patchset #10 (id:230001) as https://chromium.googlesource.com/v8/v8/+/de04df74129bba5539c6b887a2d2c81015f... |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
