Yep, I remember that. I was curious why in this place masm -> is used---it doesn't look like size-dependent.
Anyway, thanks for detailed explanation. yours, anton. On Sat, Apr 24, 2010 at 6:26 AM, Erik Corry <[email protected]> wrote: > > > 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 -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
