[v8-dev] Re: [Interpreter] Add support for JS calls. (issue 1323463005 by rmcil...@chromium.org)

2015-09-17 Thread rmcilroy
On 2015/09/17 15:37:55, torbjorng wrote: This CL apparently triggers BUG=532969. Opps, sorry. Fix up for review at https://codereview.chromium.org/1351943002/. https://codereview.chromium.org/1323463005/ -- -- v8-dev mailing list v8-dev@googlegroups.com

[v8-dev] Re: [Interpreter] Add support for JS calls. (issue 1323463005 by rmcil...@chromium.org)

2015-09-11 Thread rmcilroy
Benedikt, all the ports are now complete - could you review / stamp please? On 2015/09/10 20:14:06, akos.palfi.imgtec wrote: Hi Ross - I've uploaded the mips ports: https://codereview.chromium.org/1334873002 Thanks for the port - now integrated. However, there's a test failure

[v8-dev] Re: [Interpreter] Add support for JS calls. (issue 1323463005 by rmcil...@chromium.org)

2015-09-10 Thread rmcilroy
Benedikt: I'd like to port this to the other architectures. Before I do so could you have a look and let me know if you are happy with the general approach? Thanks. https://codereview.chromium.org/1323463005/ -- -- v8-dev mailing list v8-dev@googlegroups.com

[v8-dev] Re: Continuing removing deprecated functions from cctests (issue 1331013003 by myth...@google.com)

2015-09-10 Thread rmcilroy
Looks good overall, just a couple of nits. Jochen, do you want to give it a once over too (If you are fine with the approach I'll not bother you with future ones). https://codereview.chromium.org/1331013003/diff/1/test/cctest/test-unscopables-hidden-prototype.cc File

[v8-dev] Re: Continuing removing deprecated functions from cctests (issue 1331013003 by myth...@google.com)

2015-09-10 Thread rmcilroy
can you please run tryjobs? I kicked these for Mythri (I don't think she has permission yet). https://codereview.chromium.org/1331013003/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the

[v8-dev] Re: [Interpreter] Skeleton bytecode graph builder [NOT FOR COMMIT] (issue 1291693004 by o...@chromium.org)

2015-09-10 Thread rmcilroy
Looks really great. All nits except for the comment about TwoParameterTest, but the rest lgtm! https://codereview.chromium.org/1291693004/diff/260001/src/compiler/bytecode-graph-builder.cc File src/compiler/bytecode-graph-builder.cc (right):

[v8-dev] Re: Continuing removing deprecated functions from cctests (issue 1331013003 by myth...@google.com)

2015-09-10 Thread rmcilroy
lgtm, thanks! https://codereview.chromium.org/1331013003/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups "v8-dev" group. To unsubscribe from this group and stop receiving

[v8-dev] Re: Continuing removing deprecated functions from cctests (issue 1331013003 by myth...@google.com)

2015-09-10 Thread rmcilroy
https://codereview.chromium.org/1331013003/diff/1/test/cctest/test-unscopables-hidden-prototype.cc File test/cctest/test-unscopables-hidden-prototype.cc (right): https://codereview.chromium.org/1331013003/diff/1/test/cctest/test-unscopables-hidden-prototype.cc#newcode62

[v8-dev] Re: [Interpreter] Skeleton bytecode graph builder [NOT FOR COMMIT] (issue 1291693004 by o...@chromium.org)

2015-09-10 Thread rmcilroy
/interpreter/bytecode-array-iterator.cc:67: return FixedArray::get(constants, GetIndexOperand(operand_index)); On 2015/09/10 13:59:06, oth wrote: On 2015/09/10 10:19:43, rmcilroy wrote: > this should be "constants->get(GetIndexOperand(operand_index));" I think. The static

[v8-dev] Re: [Interpreter] Skeleton bytecode graph builder [NOT FOR COMMIT] (issue 1291693004 by o...@chromium.org)

2015-09-10 Thread rmcilroy
/interpreter/bytecode-array-iterator.cc:67: return FixedArray::get(constants, GetIndexOperand(operand_index)); On 2015/09/10 13:59:06, oth wrote: On 2015/09/10 10:19:43, rmcilroy wrote: > this should be "constants->get(GetIndexOperand(operand_index));" I think. The static

[v8-dev] Re: [Interpreter] Add support for JS calls. (issue 1323463005 by rmcil...@chromium.org)

2015-09-10 Thread rmcilroy
Thanks for the reviews, I'll port to ia32, arm and arm64 now. +v8-mips-ports: Could you port for mips/mips64 please? https://codereview.chromium.org/1323463005/diff/60001/src/interpreter/bytecode-array-builder.h File src/interpreter/bytecode-array-builder.h (right):

[v8-dev] Re: [Interpreter] Add support for property store operations. (issue 1319833004 by rmcil...@chromium.org)

2015-09-09 Thread rmcilroy
Ah, you can ignore my comment about SLOPPY/STRICT because I see elsewhere that you are only handling SLOPPY for now. Yes, we only handle SLOPPY mode for now. As discussed offline we will probably have to have Strict/Strong versions of these bytecodes, but I'll do that in a future CL.

[v8-dev] Re: [Interpreter] Add support for property store operations. (issue 1319833004 by rmcil...@chromium.org)

2015-09-09 Thread rmcilroy
https://codereview.chromium.org/1319833004/diff/1/src/interpreter/bytecodes.h File src/interpreter/bytecodes.h (right): https://codereview.chromium.org/1319833004/diff/1/src/interpreter/bytecodes.h#newcode101 src/interpreter/bytecodes.h:101: int index() const { return index_; } On 2015/09/09

[v8-dev] Re: [Interpreter] Ensure that implicit return undefined is generated. (issue 1308693014 by rmcil...@chromium.org)

2015-09-08 Thread rmcilroy
Reviewers: oth, Michael Starzinger, Message: Orion, please have a look, thanks. Michi for owner stamp (comments welcome) Description: [Interpreter] Ensure that implicit return undefined is generated. When there is no explicit return we need to generate an implicit return undefined.

[v8-dev] Re: [Interpreter] Add support for property store operations. (issue 1319833004 by rmcil...@chromium.org)

2015-09-08 Thread rmcilroy
Reviewers: oth, Michael Starzinger, mvstanton, Message: Michi / Orion, please review interpreter parts Michael for vector stuff. Thanks! Description: [Interpreter] Add support for property store operations. Adds support for property store operations via Store/KeyedStore ICs. Adds the

[v8-dev] Re: [Interpreter] Ensure that implicit return undefined is generated. (issue 1308693014 by rmcil...@chromium.org)

2015-09-08 Thread rmcilroy
I am not an owner of anything in this CL, but I am happy to offer my thoughts ... Sure, but Orion is not a committer yet and I need a committer stamp :). Your thoughts are much appreciated! https://codereview.chromium.org/1308693014/diff/1/src/interpreter/bytecode-generator.cc File

[v8-dev] Re: [Interpreter] Add support for JS calls. (issue 1323463005 by rmcil...@chromium.org)

2015-09-08 Thread rmcilroy
Ok. Doing the whole handler as native code would add a whole lot more complexity (e.g., ensuring that interpreter are saved and restored properly). Instead, I've written a small builtin called PushArgsAndCall which does the pushing and then directly calls builtin::Call. This avoids the need

[v8-dev] Re: [Interpreter] Ensure that implicit return undefined is generated. (issue 1308693014 by rmcil...@chromium.org)

2015-09-08 Thread rmcilroy
An alternative would be to append "Ldar undefined; return" in ToBytecodeArray and add a comment that this needs to be addressed in the soon to start flow control activity. As long as we have a clear comment either way, in the near term, then lgtm. Added a comment in HasExplicitReturn.

[v8-dev] Re: [turbofan] Clarify comment about Parameter indexing. (issue 1329043002 by mstarzin...@chromium.org)

2015-09-07 Thread rmcilroy
Ahh yes, it was the variable indexes which had the '-1', that explains my confusion. Thanks! lgtm. https://codereview.chromium.org/1329043002/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to

[v8-dev] Re: [Interpreter] Skeleton bytecode graph builder [NOT FOR COMMIT] (issue 1291693004 by o...@chromium.org)

2015-09-07 Thread rmcilroy
On 2015/09/04 16:46:58, Michael Starzinger wrote: https://codereview.chromium.org/1291693004/diff/180001/src/compiler/bytecode-graph-builder.cc File src/compiler/bytecode-graph-builder.cc (right):

[v8-dev] Re: [Interpreter] Add support for JS calls. (issue 1323463005 by rmcil...@chromium.org)

2015-09-07 Thread rmcilroy
On 2015/09/07 05:14:30, Benedikt Meurer wrote: Just two very early comments, and one question: You mentioned that it should be possible to write certain bytecode handlers as native builtins. How about doing that for the call opcode? Because that seems like a lot of machinery in TurboFan to

[v8-dev] Re: [stubs] Unify the various versions of [[Call]] with CallCallableStub. (issue 1311013008 by bmeu...@chromium.org)

2015-09-07 Thread rmcilroy
It's just a regular code object. You need to provide a Callable for it in CodeFactory. Though this one might be a bit special since it takes a variable number of arguments (which tho should be hidden by your CallVarArgs). I thought this might be the case, thanks (just wanted to makes sure since

[v8-dev] Re: [stubs] Unify the various versions of [[Call]] with CallCallableStub. (issue 1311013008 by bmeu...@chromium.org)

2015-09-07 Thread rmcilroy
Overall looks good, should be something that can be used by the Interpreter. I tried to figure out how to modify my CL to call this instead of the JSFunction code object directly, however I couldn't find a way to call a non-JS builtin from TF - is this not possible or am I missing something

[v8-dev] Re: [stubs] Unify the various versions of [[Call]] with CallCallableStub. (issue 1311013008 by bmeu...@chromium.org)

2015-09-07 Thread rmcilroy
Just tried this with the interpreter CallJS WIP CL and it works fine. LGTM, thanks! https://codereview.chromium.org/1311013008/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups

[v8-dev] Re: Start removing deprecated APIs from cctest (issue 1333463002 by joc...@chromium.org)

2015-09-07 Thread rmcilroy
lgtm with an optional suggestion. Thanks! https://codereview.chromium.org/1333463002/diff/1/test/cctest/cctest.h File test/cctest/cctest.h (right): https://codereview.chromium.org/1333463002/diff/1/test/cctest/cctest.h#newcode452 test/cctest/cctest.h:452: return v8::Local(); nit - could you

[v8-dev] Re: [Interpreter] Skeleton bytecode graph builder [NOT FOR COMMIT] (issue 1291693004 by o...@chromium.org)

2015-09-04 Thread rmcilroy
I like the iterator :). A few more comments, I'll look in more depth when the tests are there. https://codereview.chromium.org/1291693004/diff/160001/src/compiler/bytecode-graph-builder.cc File src/compiler/bytecode-graph-builder.cc (right):

[v8-dev] [Interpreter] Add support for JS calls. (issue 1323463005 by rmcil...@chromium.org)

2015-09-04 Thread rmcilroy
Reviewers: Benedikt Meurer, Message: Hi Benedikt. Here is an in-progress CL which adds support for JS calls to the interpreter as well as adding support for a CallVarArgs instruction in TF as discussed offline today. Currently it assumes the callable is a JSFunction and calls it directly,

[v8-dev] Re: Remove GC metadata of code object before serializing. (issue 1313953008 by o...@chromium.org)

2015-09-03 Thread rmcilroy
lgtm, thanks! https://codereview.chromium.org/1313953008/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups "v8-dev" group. To unsubscribe from this group and stop receiving

[v8-dev] Re: [Interpreter] Skeleton bytecode graph builder [NOT FOR COMMIT] (issue 1291693004 by o...@chromium.org)

2015-09-03 Thread rmcilroy
Looking really good, mostly just nits with one optional suggestion. https://codereview.chromium.org/1291693004/diff/140001/src/compiler/bytecode-graph-builder.cc File src/compiler/bytecode-graph-builder.cc (right):

[v8-dev] Re: Move register-operand and parameter-operand conversion routines into (issue 1325983002 by o...@chromium.org)

2015-09-02 Thread rmcilroy
Still lgtm, thanks! https://codereview.chromium.org/1325983002/diff/40001/src/interpreter/bytecodes.cc File src/interpreter/bytecodes.cc (right): https://codereview.chromium.org/1325983002/diff/40001/src/interpreter/bytecodes.cc#newcode183 src/interpreter/bytecodes.cc:183:

[v8-dev] Re: Move register-operand and parameter-operand conversion routines into (issue 1325983002 by o...@chromium.org)

2015-09-02 Thread rmcilroy
Looks much cleaner, thanks. Lgtm once comments are addressed. https://codereview.chromium.org/1325983002/diff/40001/src/interpreter/bytecodes.cc File src/interpreter/bytecodes.cc (right): https://codereview.chromium.org/1325983002/diff/40001/src/interpreter/bytecodes.cc#newcode173

[v8-dev] Re: [Interpreter] Add support for property load operations. (issue 1309843007 by rmcil...@chromium.org)

2015-09-02 Thread rmcilroy
https://codereview.chromium.org/1309843007/diff/60001/src/interpreter/bytecode-generator.cc File src/interpreter/bytecode-generator.cc (right): https://codereview.chromium.org/1309843007/diff/60001/src/interpreter/bytecode-generator.cc#newcode318 src/interpreter/bytecode-generator.cc:318:

[v8-dev] Re: Move register-operand and parameter-operand conversion routines into (issue 1325983002 by o...@chromium.org)

2015-09-02 Thread rmcilroy
Looks good. My main suggestion is whether we can move Register class to bytecodes.h to keep all the logic together a bit more. https://codereview.chromium.org/1325983002/diff/20001/src/interpreter/bytecode-array-builder.h File src/interpreter/bytecode-array-builder.h (right):

[v8-dev] Re: Code::WipeOutHeader needs to null out the next code link to avoid (issue 1310503006 by o...@chromium.org)

2015-09-02 Thread rmcilroy
lgtm with a question for Hannes. https://codereview.chromium.org/1310503006/diff/1/src/objects-inl.h File src/objects-inl.h (right): https://codereview.chromium.org/1310503006/diff/1/src/objects-inl.h#newcode6420 src/objects-inl.h:6420: WRITE_FIELD(this, kNextCodeLinkOffset, NULL); +hpayer

[v8-dev] Re: [Interpreter] Skeleton bytecode graph builder [NOT FOR COMMIT] (issue 1291693004 by o...@chromium.org)

2015-09-02 Thread rmcilroy
* jsgraph); On 2015/09/01 15:25:57, oth wrote: On 2015/09/01 14:00:06, rmcilroy wrote: > Could we just have the one constructor and one CreateGraph and have the test > framework do the necessary work to create an appropriate CompilationInfo rather > than having test specific ent

[v8-dev] Re: [Interpreter] Add support for property load operations. (issue 1309843007 by rmcil...@chromium.org)

2015-09-01 Thread rmcilroy
Reviewers: Michael Starzinger, mvstanton, oth, Message: Adds support for loadICs to the interpreter. Michi for TF stuff Michael for TypeFeedbackVector stuff Orion for bytecode-generator/array-builder stuff PTAL, thanks! Description: [Interpreter] Add support for property load operations.

[v8-dev] Re: [Interpreter] Skeleton bytecode graph builder [NOT FOR COMMIT] (issue 1291693004 by o...@chromium.org)

2015-09-01 Thread rmcilroy
Looking good to me. I don't have enough knowledge on TF to comment on the graph building questions, but made a couple of readability nits. https://codereview.chromium.org/1291693004/diff/80001/src/compiler/bytecode-graph-builder.cc File src/compiler/bytecode-graph-builder.cc (right):

[v8-dev] Re: [Interpreter] Add support for property load operations. (issue 1309843007 by rmcil...@chromium.org)

2015-09-01 Thread rmcilroy
https://codereview.chromium.org/1309843007/diff/20001/src/interpreter/bytecode-array-builder.cc File src/interpreter/bytecode-array-builder.cc (right): https://codereview.chromium.org/1309843007/diff/20001/src/interpreter/bytecode-array-builder.cc#newcode303

[v8-dev] Re: Use ShouldEnsureSpaceForLazyDeopt more. (issue 1310283005 by tit...@chromium.org)

2015-08-28 Thread rmcilroy
lgtm, thanks! https://codereview.chromium.org/1310283005/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving

[v8-dev] Re: [Interpreter] Add support for loading literals from the constant pool. (issue 1321663003 by rmcil...@chromium.org)

2015-08-28 Thread rmcilroy
://codereview.chromium.org/1321663003/diff/40001/test/cctest/interpreter/test-bytecode-generator.cc#newcode257 test/cctest/interpreter/test-bytecode-generator.cc:257: { 3.14, 3.14 } On 2015/08/27 19:36:29, Michael Starzinger wrote: On 2015/08/27 16:34:07, rmcilroy wrote: Michi - looks like double literals aren't de

[v8-dev] Re: [heap] Move IdentityMap data structure out of heap. (issue 1320503004 by mstarzin...@chromium.org)

2015-08-28 Thread rmcilroy
lgtm, thanks! https://codereview.chromium.org/1320503004/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving

[v8-dev] Re: [Interpreter] Add support for loading literals from the constant pool. (issue 1321663003 by rmcil...@chromium.org)

2015-08-28 Thread rmcilroy
Review comments addressed, PTAL, thanks. https://codereview.chromium.org/1321663003/diff/40001/src/DEPS File src/DEPS (right): https://codereview.chromium.org/1321663003/diff/40001/src/DEPS#newcode8 src/DEPS:8: +src/heap/identity-map.h, Done (thanks for doing the move!)

[v8-dev] Re: [Interpreter] Add support for loading literals from the constant pool. (issue 1321663003 by rmcil...@chromium.org)

2015-08-27 Thread rmcilroy
Reviewers: Michael Starzinger, Message: Michi, could you take a look please? Thanks. https://codereview.chromium.org/1321663003/diff/40001/test/cctest/interpreter/test-bytecode-generator.cc File test/cctest/interpreter/test-bytecode-generator.cc (right):

[v8-dev] Re: [Interpreter] Add support for parameter variables. (issue 1303403004 by rmcil...@chromium.org)

2015-08-27 Thread rmcilroy
https://codereview.chromium.org/1303403004/diff/140001/src/arm64/builtins-arm64.cc File src/arm64/builtins-arm64.cc (right): https://codereview.chromium.org/1303403004/diff/140001/src/arm64/builtins-arm64.cc#newcode1032 src/arm64/builtins-arm64.cc:1032: __ Drop(x1); On 2015/08/26 13:44:36,

[v8-dev] Re: [interpreter] Add constant_pool() to BytecodeArray. (issue 1314953004 by rmcil...@chromium.org)

2015-08-27 Thread rmcilroy
https://codereview.chromium.org/1314953004/diff/20001/test/cctest/test-heap.cc File test/cctest/test-heap.cc (right): https://codereview.chromium.org/1314953004/diff/20001/test/cctest/test-heap.cc#newcode710 test/cctest/test-heap.cc:710: i::FLAG_always_compact = true; On 2015/08/26 22:19:49,

[v8-dev] Re: Move (uppercase) JS builtins from js builtins object to native context. (issue 1316943002 by yang...@chromium.org)

2015-08-27 Thread rmcilroy
Interpreter LGTM with one nit. https://codereview.chromium.org/1316943002/diff/1/test/unittests/compiler/interpreter-assembler-unittest.cc File test/unittests/compiler/interpreter-assembler-unittest.cc (right):

[v8-dev] Re: Make FlushICache NOP for Nvidia Denver 1.0 only (issue 1287173004 by sbo...@nvidia.com)

2015-08-27 Thread rmcilroy
+hablich for merge request (see comments above). https://codereview.chromium.org/1287173004/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from

[v8-dev] Re: Move (uppercase) JS builtins from js builtins object to native context. (issue 1316943002 by yang...@chromium.org)

2015-08-26 Thread rmcilroy
Interpreter changes look good to me. https://codereview.chromium.org/1316943002/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group

[v8-dev] Re: [interpreter] Add constant_pool() to BytecodeArray. (issue 1314953004 by rmcil...@chromium.org)

2015-08-26 Thread rmcilroy
Reviewers: Hannes Payer, Message: Hannes, could you take a look please? Please let me know if I've missed any important visitors with this change. Michi for FYI since I plan to send you the followup CL (but comments welcome). Description: [interpreter] Add constant_pool() to BytecodeArray.

[v8-dev] Re: [Interpreter] Add support for parameter variables. (issue 1303403004 by rmcil...@chromium.org)

2015-08-25 Thread rmcilroy
Reviewers: Michael Starzinger, Message: Michael, could you take a look please, thanks. Description: [Interpreter] Add support for parameter variables. Adds support for parameters to the BytecodeArrayBuilder and BytecodeGenerator. Parameters are accessed as negative interpreter registers.

[v8-dev] Re: [interpreter] Allow verification and trace-turbo for bytecode handlers. (issue 1308863004 by rmcil...@chromium.org)

2015-08-25 Thread rmcilroy
Ping? https://codereview.chromium.org/1308863004/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from

[v8-dev] Re: [Interpreter] Pass context to interpreter bytecode handlers and add LoadConstextSlot (issue 1294133004 by rmcil...@chromium.org)

2015-08-24 Thread rmcilroy
https://codereview.chromium.org/1294133004/diff/20001/src/compiler/linkage.cc File src/compiler/linkage.cc (right): https://codereview.chromium.org/1294133004/diff/20001/src/compiler/linkage.cc#newcode416 src/compiler/linkage.cc:416: #if defined(V8_TARGET_ARCH_IA32) On 2015/08/20 09:32:02,

[v8-dev] Re: [Interpreter] Pass context to interpreter bytecode handlers and add LoadConstextSlot (issue 1294133004 by rmcil...@chromium.org)

2015-08-24 Thread rmcilroy
On 2015/08/20 09:32:02, Michael Starzinger wrote: LGTM. As discussed offline: I am fine with landing this as it is to unblock interpreter work. We should just keep the alternative ideas about context chain access in mind. This might mean that we'd potentially remove this parameter again if

[v8-dev] Re: [Interpreter] Add implementations of arithmetic binary op bytecodes. (issue 1300813005 by rmcil...@chromium.org)

2015-08-24 Thread rmcilroy
Comments addressed, PTAL. https://codereview.chromium.org/1300813005/diff/1/src/compiler/interpreter-assembler.cc File src/compiler/interpreter-assembler.cc (right): https://codereview.chromium.org/1300813005/diff/1/src/compiler/interpreter-assembler.cc#newcode225

[v8-dev] Re: [turbofan] Add control and effect inputs to RawMachineAssembler calls. (issue 1283193007 by rmcil...@chromium.org)

2015-08-21 Thread rmcilroy
On 2015/08/21 10:31:52, titzer wrote: *sigh* I'll be ok with this patch, but keep in mind that the graphs built by raw machine assembler are really broken and don't make sense at all without a schedule. Noted, thanks. PTAL.

[v8-dev] Re: [Interpreter] Allow verification and trace-turbo for bytecode handlers. (issue 1297203002 by rmcil...@chromium.org)

2015-08-21 Thread rmcilroy
Updated to replace !info-IsStub() !info-IsBytecodeHandler() with a new ShouldEnsureSpaceForLazyDeopt() function based on offline discussion with Danno. Ben, could you PTAL? Thanks. https://codereview.chromium.org/1297203002/ -- -- v8-dev mailing list v8-dev@googlegroups.com

[v8-dev] Re: [Interpreter] Allow verification and trace-turbo for bytecode handlers. (issue 1297203002 by rmcil...@chromium.org)

2015-08-21 Thread rmcilroy
+Yang for serializer/ review. Ben: I've reworked things to have a GetDebugName on CompilerInfo. I've replaced all the copy/pasted code which got the debug name from the CompilerInfo with a call to this new function and have done a couple of other cleanups (removing a useless parameter on

[v8-dev] [interpreter] Allow verification and trace-turbo for bytecode handlers. (issue 1308863004 by rmcil...@chromium.org)

2015-08-21 Thread rmcilroy
InterpreterAssembler::GenerateCode() { End(); + const char* bytecode_name = interpreter::Bytecodes::ToString(bytecode_); Schedule* schedule = raw_assembler_-Export(); // TODO(rmcilroy): use a non-testing code generator. - HandleCode code = Pipeline::GenerateCodeForTesting

[v8-dev] Re: [Interpreter] Pass context to interpreter bytecode handlers and add LoadConstextSlot (issue 1294133004 by rmcil...@chromium.org)

2015-08-20 Thread rmcilroy
On 2015/08/19 16:41:52, Michael Starzinger wrote: On 2015/08/19 15:42:16, rmcilroy wrote: On 2015/08/19 15:15:37, Michael Starzinger wrote: High level question: Do we need to keep the context in a machine register? Or could be but it into one of the interpreter registers thereby avoiding

[v8-dev] Re: Turn v8.h into a normal header. (issue 1293593005 by mstarzin...@chromium.org)

2015-08-20 Thread rmcilroy
On 2015/08/20 05:34:55, Benedikt Meurer wrote: LGTM!! Lgtm https://codereview.chromium.org/1293593005/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To

[v8-dev] Re: [Interpreter] Pass context to interpreter bytecode handlers and add LoadConstextSlot (issue 1294133004 by rmcil...@chromium.org)

2015-08-19 Thread rmcilroy
Reviewers: Michael Starzinger, oth, Message: Michael / Orion, PTAL, thanks. This is required for calling the Add/Sub/Div/Mod JS builtins for the respective bytecode implementations (the next CL I'll send out). Description: [Interpreter] Pass context to interpreter bytecode handlers and add

[v8-dev] Re: [Interpreter] Add implementations of arithmetic binary op bytecodes. (issue 1300813005 by rmcil...@chromium.org)

2015-08-19 Thread rmcilroy
Reviewers: Michael Starzinger, oth, Message: Michael / Orion, could you please take a look, thanks. Description: [Interpreter] Add implementations of arithmetic binary op bytecodes. Adds implementations and tests for the following bytecodes: - Add - Sub - Mul - Div - Mod Also adds

[v8-dev] Re: [Interpreter] Pass context to interpreter bytecode handlers and add LoadConstextSlot (issue 1294133004 by rmcil...@chromium.org)

2015-08-19 Thread rmcilroy
On 2015/08/19 15:15:37, Michael Starzinger wrote: High level question: Do we need to keep the context in a machine register? Or could be but it into one of the interpreter registers thereby avoiding pinning yet another machine register. I don't want to have it in an interpreter Register,

[v8-dev] Re: [turbofan] Add control and effect inputs to RawMachineAssembler calls. (issue 1283193007 by rmcil...@chromium.org)

2015-08-19 Thread rmcilroy
On 2015/08/19 15:16:17, titzer wrote: On 2015/08/19 06:55:32, rmcilroy wrote: Ben, please take a look, thanks. What's the motivation for this? The graphs built by the raw assembler already have no hope of being verified because they don't have proper effect chains nor control chains

[v8-dev] Re: [turbofan] Add control and effect inputs to RawMachineAssembler calls. (issue 1283193007 by rmcil...@chromium.org)

2015-08-19 Thread rmcilroy
Reviewers: titzer, Message: Ben, please take a look, thanks. Description: [turbofan] Add control and effect inputs to RawMachineAssembler calls. Calls should have control and effect inputs, which were previously missing for RawMachineAssembler call operations. Add control and effect edges to

[v8-dev] Re: [Interpreter] Minimal bytecode generator. (issue 1294543002 by o...@chromium.org)

2015-08-18 Thread rmcilroy
lgtm with a couple of nits. https://codereview.chromium.org/1294543002/diff/30001/test/cctest/interpreter/test-bytecode-generator.cc File test/cctest/interpreter/test-bytecode-generator.cc (right):

[v8-dev] Re: [interpreter]: Changes to interpreter builtins for accumulator and register file registers. (issue 1289863003 by rmcil...@chromium.org)

2015-08-18 Thread rmcilroy
Ping? https://codereview.chromium.org/1289863003/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from

[v8-dev] Re: [Interpreter] Allow verification and trace-turbo for bytecode handlers. (issue 1297203002 by rmcil...@chromium.org)

2015-08-18 Thread rmcilroy
Reviewers: titzer, Message: These are the changes needed to print out bytecode handlers to the turbofan visualizer. I'm happy to do this differently if you need similar things for WASM (e.g., you were talking about using an enum for the pipeline steps taken in today's meeting). PTAL, thanks.

[v8-dev] Re: [interpreter]: Changes to interpreter builtins for accumulator and register file registers. (issue 1289863003 by rmcil...@chromium.org)

2015-08-18 Thread rmcilroy
https://codereview.chromium.org/1289863003/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from it, send

[v8-dev] Re: [interpreter]: Changes to interpreter builtins for accumulator and register file registers. (issue 1289863003 by rmcil...@chromium.org)

2015-08-18 Thread rmcilroy
https://codereview.chromium.org/1289863003/diff/60001/src/compiler/interpreter-assembler.h File src/compiler/interpreter-assembler.h (right): https://codereview.chromium.org/1289863003/diff/60001/src/compiler/interpreter-assembler.h#newcode96 src/compiler/interpreter-assembler.h:96: Node*

[v8-dev] Re: [Interpreter] Add implementations for load immediate bytecodes. (issue 1294793002 by rmcil...@chromium.org)

2015-08-18 Thread rmcilroy
https://codereview.chromium.org/1294793002/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from it, send

[v8-dev] Re: [Interpreter] Add implementations for load immediate bytecodes. (issue 1294793002 by rmcil...@chromium.org)

2015-08-18 Thread rmcilroy
https://codereview.chromium.org/1294793002/diff/20001/src/compiler/interpreter-assembler.cc File src/compiler/interpreter-assembler.cc (right): https://codereview.chromium.org/1294793002/diff/20001/src/compiler/interpreter-assembler.cc#newcode137 src/compiler/interpreter-assembler.cc:137: Node*

[v8-dev] Re: [Interpreter] Minimal bytecode generator. (issue 1294543002 by o...@chromium.org)

2015-08-14 Thread rmcilroy
Looks great to me (once tests are added). Maybe let Michael have a look too? For tests, how about adding a cctest (we probably do this as a unittests) which compiles JS source and checks if the resulting bytecode is as expected?

[v8-dev] Re: [interpreter]: Changes to interpreter builtins for accumulator and register file registers. (issue 1289863003 by rmcil...@chromium.org)

2015-08-14 Thread rmcilroy
Thanks for the review. Comments addressed, PTAL. https://codereview.chromium.org/1289863003/diff/20001/src/compiler/interpreter-assembler.h File src/compiler/interpreter-assembler.h (right): https://codereview.chromium.org/1289863003/diff/20001/src/compiler/interpreter-assembler.h#newcode40

[v8-dev] [Interpreter] Add implementations for load immediate bytecodes. (issue 1294793002 by rmcil...@chromium.org)

2015-08-14 Thread rmcilroy
Reviewers: Michael Starzinger, oth, Message: Michael, Orion, could you take a look please? Thanks. Description: [Interpreter] Add implementations for load immediate bytecodes. Adds implementations and tests for the following bytecodes: - LdaZero - LdaSmi8 - LdaUndefined - LdaNull -

[v8-dev] [Interpreter] Move interpreter initialization until after snapshot deserialization. (issue 1290883004 by rmcil...@chromium.org)

2015-08-14 Thread rmcilroy
Reviewers: Hannes Payer, Message: Hannes: could you please take a look, thanks. Description: [Interpreter] Move interpreter initialization until after snapshot deserialization. The interpreter needs to be initialized after the snapshot has been deserialized. BUG=v8:4280 LOG=N Please review

[v8-dev] [interpreter]: Changes to interpreter builtins for accumulator and register file registers. (issue 1289863003 by rmcil...@chromium.org)

2015-08-14 Thread rmcilroy
Reviewers: titzer, Message: Ben: Could you take a look please? Thanks. Description: [interpreter]: Changes to interpreter builtins for accumulator and register file registers. Makes the following modifications to the interpreter builtins and InterpreterAssembler: - Adds an accumulator

[v8-dev] Re: MIPS64: Fix InterpreterEntryTrampoline(). (issue 1286343002 by paul.l...@imgtec.com)

2015-08-13 Thread rmcilroy
On 2015/08/13 04:38:58, paul.l... wrote: Fix a mistake in my porting of https://codereview.chromium.org/1245133002 for mips. PTAL. Lgtm, thanks! https://codereview.chromium.org/1286343002/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You

[v8-dev] Re: [interpreter]: Update BytecodeArrayBuilder register handling. (issue 1283313003 by rmcil...@chromium.org)

2015-08-13 Thread rmcilroy
Reviewers: oth, Message: Orion: could you take a look please? Thanks. Description: [interpreter]: Update BytecodeArrayBuilder register handling. Modifies the BytecodeArrayBuilder to create register operands which are negative. This reduces the number of instructions to access registers by the

[v8-dev] Re: [interpreter]: Update BytecodeArrayBuilder register handling. (issue 1283313003 by rmcil...@chromium.org)

2015-08-13 Thread rmcilroy
Micheal: could you stamp for committer/OWNERS please? Thanks. https://codereview.chromium.org/1283313003/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To

[v8-dev] Re: [interpreter]: Update BytecodeArrayBuilder register handling. (issue 1283313003 by rmcil...@chromium.org)

2015-08-13 Thread rmcilroy
https://codereview.chromium.org/1283313003/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from it, send

[v8-dev] Re: [interpreter]: Update BytecodeArrayBuilder register handling. (issue 1283313003 by rmcil...@chromium.org)

2015-08-13 Thread rmcilroy
https://codereview.chromium.org/1283313003/diff/40001/src/interpreter/bytecode-array-builder.h File src/interpreter/bytecode-array-builder.h (right): https://codereview.chromium.org/1283313003/diff/40001/src/interpreter/bytecode-array-builder.h#newcode82

[v8-dev] [interpreter]: Fix interpreter handler table initialization. (issue 1288893003 by rmcil...@chromium.org)

2015-08-13 Thread rmcilroy
Reviewers: oth, Hannes Payer, Message: Orion: could you take a look please? Hannes: could you stamp for OWNER/committer (any comments welcome too). Description: [interpreter]: Fix interpreter handler table initialization. BUG=v8:4280 LOG=N Please review this at

[v8-dev] Re: [interpreter]: Fix interpreter handler table initialization. (issue 1288893003 by rmcil...@chromium.org)

2015-08-13 Thread rmcilroy
To give some background on why this is needed - turns out the builtins aren't generated at the time CreateUninitializedInterpreterTable is called. https://codereview.chromium.org/1288893003/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You

[v8-dev] Re: [interpreter]: Fix interpreter handler table initialization. (issue 1288893003 by rmcil...@chromium.org)

2015-08-13 Thread rmcilroy
lgtm. https://codereview.chromium.org/1288893003/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from

[v8-dev] Re: [interpreter]: Fix interpreter handler table initialization. (issue 1288893003 by rmcil...@chromium.org)

2015-08-13 Thread rmcilroy
On 2015/08/13 13:59:32, rmcilroy wrote: lgtm. Hannes: could you stamp for OWNERS please? https://codereview.chromium.org/1288893003/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google

[v8-dev] Re: [Interpreter] Register conversion fix and test. (issue 1294523002 by o...@chromium.org)

2015-08-13 Thread rmcilroy
On 2015/08/13 15:54:24, oth wrote: Ross, PTAL, thanks! lgtm, thanks for the fix! https://codereview.chromium.org/1294523002/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups

[v8-dev] Re: [interpreter] Fix nosnap build for interpreter table generation. (issue 1278413002 by rmcil...@chromium.org)

2015-08-10 Thread rmcilroy
https://codereview.chromium.org/1278413002/diff/40001/src/interpreter/interpreter.cc File src/interpreter/interpreter.cc (right): https://codereview.chromium.org/1278413002/diff/40001/src/interpreter/interpreter.cc#newcode43 src/interpreter/interpreter.cc:43: if (initialized_) return; On

[v8-dev] Re: [interpreter] Fix nosnap build for interpreter table generation. (issue 1278413002 by rmcil...@chromium.org)

2015-08-10 Thread rmcilroy
create_heap_objects) { On 2015/08/10 16:59:25, Hannes Payer wrote: On 2015/08/10 16:54:36, rmcilroy wrote: On 2015/08/10 16:50:00, Hannes Payer wrote: Why would you call this function with create_heap_objects=false? It is called with create_heap_objects=false in Isolate::Init

[v8-dev] Re: [interpreter] Fix nosnap build for interpreter table generation. (issue 1278413002 by rmcil...@chromium.org)

2015-08-10 Thread rmcilroy
https://codereview.chromium.org/1278413002/diff/60001/src/interpreter/interpreter.cc File src/interpreter/interpreter.cc (right): https://codereview.chromium.org/1278413002/diff/60001/src/interpreter/interpreter.cc#newcode41 src/interpreter/interpreter.cc:41: void Interpreter::Initialize(bool

[v8-dev] [interpreter] Fix nosnap build for interpreter table generation. (issue 1278413002 by rmcil...@chromium.org)

2015-08-10 Thread rmcilroy
Reviewers: Hannes Payer, Message: Hannes, could you have a quick look please (this is needed to green the tree). Thanks! Description: [interpreter] Fix nosnap build for interpreter table generation. Moves the creation of the interpreter table early on during initialization to ensure that

[v8-dev] [interpreter] Fix nosnap build for interpreter table generation. (issue 1278413002 by rmcil...@chromium.org)

2015-08-10 Thread rmcilroy
Reviewers: Hannes Payer, Message: Hannes, could you have a quick look please (this is needed to green the tree). Thanks! Description: [interpreter] Fix nosnap build for interpreter table generation. Moves the creation of the interpreter table early on during initialization to ensure that

[v8-dev] Re: [interpreter] Adds interpreter cctests. (issue 1269683002 by rmcil...@chromium.org)

2015-07-31 Thread rmcilroy
PTAL, thanks. https://codereview.chromium.org/1269683002/diff/60001/test/cctest/interpreter/test-interpreter.cc File test/cctest/interpreter/test-interpreter.cc (right): https://codereview.chromium.org/1269683002/diff/60001/test/cctest/interpreter/test-interpreter.cc#newcode31

[v8-dev] Re: [interpreter] Adds interpreter cctests. (issue 1269683002 by rmcil...@chromium.org)

2015-07-31 Thread rmcilroy
Ben, could you take a look please, thanks. https://codereview.chromium.org/1269683002/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this

[v8-dev] Re: [interpreter] Adds interpreter cctests. (issue 1269683002 by rmcil...@chromium.org)

2015-07-31 Thread rmcilroy
Ben, could you take a look please, thanks. https://codereview.chromium.org/1269683002/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this

[v8-dev] [turbofan] Fix kArchTailCallCodeObject on ia32/x64. (issue 1265723003 by rmcil...@chromium.org)

2015-07-31 Thread rmcilroy
Reviewers: titzer, Message: PTAL. As discussed, this will be tested byhttps://codereview.chromium.org/1269683002/, but I will add an explicit test when I return from vacation on Aug 10th. Description: [turbofan] Fix kArchTailCallCodeObject on ia32/x64. Previously these instructions tried to

[v8-dev] Re: [Intepreter] Naive bytecode generation local variable assignment and +/*-. (issue 1266713004 by o...@chromium.org)

2015-07-31 Thread rmcilroy
://codereview.chromium.org/1266713004/diff/20001/src/interpreter/bytecode-array-builder.cc#newcode76 src/interpreter/bytecode-array-builder.cc:76: int BytecodeArrayBuilder::AllocateScratchRegister() { On 2015/07/30 15:38:42, oth wrote: On 2015/07/30 10:12:24, rmcilroy (OOO until 10th Aug) wrote: These should be Pop

[v8-dev] Re: [Interpreter] Remove unnecessary const specifiers on scalar types. (issue 1269813006 by o...@chromium.org)

2015-07-31 Thread rmcilroy
On 2015/07/31 10:01:43, oth wrote: PTAL, thanks! lgtm, thanks! https://codereview.chromium.org/1269813006/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To

[v8-dev] Re: Fix idle notification for background tab. (issue 1269583002 by u...@chromium.org)

2015-07-30 Thread rmcilroy
: idle_times_which_made_no_progress_++; On 2015/07/29 18:21:30, ulan wrote: On 2015/07/29 18:11:04, rmcilroy (OOO until 10th Aug) wrote: Could this cause wrapping of idle_times_which_made_no_progress_? Maybe clap idle_times_which_made_no_progress_ to be less than or equal to kMaxNoProgressIdleTimes? Thanks, rewrote to clap

[v8-dev] Re: [interpreter] Add Interpreter{Entry,Exit}Trampoline builtins. (issue 1245133002 by rmcil...@chromium.org)

2015-07-30 Thread rmcilroy
https://codereview.chromium.org/1245133002/diff/11/src/arm/builtins-arm.cc File src/arm/builtins-arm.cc (right): https://codereview.chromium.org/1245133002/diff/11/src/arm/builtins-arm.cc#newcode951 src/arm/builtins-arm.cc:951: // - Allocating a new local context if applicable. On

  1   2   3   4   5   6   7   8   >