LGTM. I can't quite figure out the logic behind where SetSourcePosition is being called. As far as I know, we need it for every statement and every IC call. In the optimizing backend, we have it for every statement and every expression where an IC call is possible.
It looks like you are putting it in every expression where an IC call is inevitable. In general, I like that, but then it can be removed from places like VisitAssignment. http://codereview.chromium.org/496009/diff/14001/14008 File src/arm/fast-codegen-arm.cc (right): http://codereview.chromium.org/496009/diff/14001/14008#newcode917 src/arm/fast-codegen-arm.cc:917: Handle<Code> ic(Builtins::builtin(Builtins::StoreIC_Initialize)); SetSourcePosition. http://codereview.chromium.org/496009/diff/14001/14008#newcode1012 src/arm/fast-codegen-arm.cc:1012: Move(context, r3); Indentation looks off here. http://codereview.chromium.org/496009/diff/14001/14008#newcode1042 src/arm/fast-codegen-arm.cc:1042: Handle<Code> ic(Builtins::builtin(Builtins::StoreIC_Initialize)); SetSourcePosition. http://codereview.chromium.org/496009/diff/14001/14008#newcode1072 src/arm/fast-codegen-arm.cc:1072: Handle<Code> ic(Builtins::builtin(Builtins::KeyedStoreIC_Initialize)); SetSourcePosition. http://codereview.chromium.org/496009/diff/14001/14008#newcode1412 src/arm/fast-codegen-arm.cc:1412: Handle<Code> ic(Builtins::builtin(Builtins::LoadIC_Initialize)); SetSourcePosition. http://codereview.chromium.org/496009/diff/14001/14008#newcode1461 src/arm/fast-codegen-arm.cc:1461: } else { Too many spaces after else? http://codereview.chromium.org/496009/diff/14001/14008#newcode1517 src/arm/fast-codegen-arm.cc:1517: __ CallRuntime(Runtime::kNumberAdd, 2); For the INC case, we can use stm to push the two arguments to the runtime call. Didn't we decide that was better? http://codereview.chromium.org/496009/diff/14001/14008#newcode1551 src/arm/fast-codegen-arm.cc:1551: } break; We usually put break inside the braces. http://codereview.chromium.org/496009/diff/14001/14008#newcode1563 src/arm/fast-codegen-arm.cc:1563: } break; And here. http://codereview.chromium.org/496009/diff/14001/14005 File src/compiler.cc (right): http://codereview.chromium.org/496009/diff/14001/14005#newcode653 src/compiler.cc:653: // expression context state. Comment can go away. http://codereview.chromium.org/496009/diff/14001/14006 File src/fast-codegen.cc (right): http://codereview.chromium.org/496009/diff/14001/14006#newcode634 src/fast-codegen.cc:634: SetSourcePosition(expr->position()); No need? http://codereview.chromium.org/496009/diff/14001/14004 File src/ia32/fast-codegen-ia32.cc (right): http://codereview.chromium.org/496009/diff/14001/14004#newcode1527 src/ia32/fast-codegen-ia32.cc:1527: } break; Break inside braces. http://codereview.chromium.org/496009/diff/14001/14004#newcode1542 src/ia32/fast-codegen-ia32.cc:1542: } break; And here. http://codereview.chromium.org/496009 -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
