Den 23. apr. 2010 13.05 skrev <[email protected]>: > > http://codereview.chromium.org/1689010/diff/1/2 > File src/ia32/stub-cache-ia32.cc (right): > > http://codereview.chromium.org/1689010/diff/1/2#newcode1331 > src/ia32/stub-cache-ia32.cc:1331: __ j(above, &call_builtin); > just for my education: why this change? > > http://codereview.chromium.org/1689010/diff/1/2#newcode1429 > src/ia32/stub-cache-ia32.cc:1429: __ j(equal, &return_undefined); > what if there is another object in prototype chain with this element, > should'nt we return it? > > a = new Array(1) > a.__proto__[0] = 1 > assertEquals(1, a.pop()) > > but with your change I would expect undefined here, correct? > > http://codereview.chromium.org/1689010/diff/1/3 > File src/x64/codegen-x64.cc (right): > > http://codereview.chromium.org/1689010/diff/1/3#newcode7779 > src/x64/codegen-x64.cc:7779: masm->RecordWriteHelper(object_, addr_, > scratch_); > just for my education, why masm-> form instead of __ ? I know it's the > same on ia32, just asking >
At one point I built a generated code coverage tool that checked whether each line with __ generated an instruction that was run by our tests (it's not enough that the C++ line is run, the code it generates should also be run). It worked by inserting extra instructions into the generated code. But there are some places that don't work with extra instructions because the size changes (think runtime patching). So in these places I changed the source to use masm instead of __. Then the coverage tool ignores that line (it's syntax based). The profiling tool has bit rotted and should be either renovated or removed, but that's the reason. > > http://codereview.chromium.org/1689010/diff/1/5 > File src/x64/macro-assembler-x64.cc (right): > > http://codereview.chromium.org/1689010/diff/1/5#newcode198 > src/x64/macro-assembler-x64.cc:198: shr(scratch, > Immediate(kPointerSizeLog2)); > why this change? > > http://codereview.chromium.org/1689010/diff/1/6 > File src/x64/macro-assembler-x64.h (right): > > http://codereview.chromium.org/1689010/diff/1/6#newcode543 > src/x64/macro-assembler-x64.h:543: Register scratch, int save_at_depth, > do you need it in this change? that should be API calls directed > > http://codereview.chromium.org/1689010/diff/1/7 > File src/x64/stub-cache-x64.cc (right): > > http://codereview.chromium.org/1689010/diff/1/7#newcode558 > src/x64/stub-cache-x64.cc:558: // Holds information about possible > function call optimizations. > ditto is not probably needed for this change. And I suspect Pavel is > already doing this port > > http://codereview.chromium.org/1689010/diff/1/7#newcode1199 > src/x64/stub-cache-x64.cc:1199: __ j(greater, > &attempt_to_grow_elements); > above instead of greater? just asking > > http://codereview.chromium.org/1689010/diff/1/7#newcode1209 > src/x64/stub-cache-x64.cc:1209: index.reg, index.scale, > indent off > > http://codereview.chromium.org/1689010/diff/1/7#newcode1348 > src/x64/stub-cache-x64.cc:1348: __ j(equal, &return_undefined); > ditto if it's valid for elements on proto > > > http://codereview.chromium.org/1689010/show > > -- > v8-dev mailing list > [email protected] > http://groups.google.com/group/v8-dev > -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
