On Fri, Jun 17, 2011 at 9:15 PM, Kasper Lund <[email protected]> wrote: > I'll help you land it on Monday, Stephen. Thanks a lot for the patch!
Generalized a bit and landed in http://code.google.com/p/v8/source/detail?r=8323. > On Fri, Jun 17, 2011 at 8:04 PM, Stephen Adams <[email protected]> wrote: >> I have updated the fix but I'm not a v8 committer. >> >> On Tue, Jun 14, 2011 at 1:40 AM, Kevin Millikin <[email protected]> >> wrote: >>> >>> Hi Stephen, >>> OK, I agree that's a silly jump. You're right that folding in the parser >>> is simplest here (it complicates the compiler a lot to test for and handle >>> the unreachable labels after every branch, which is why we don't already >>> eliminate the dead code). >>> There is no documentation of the Crankshaft intermediate languages >>> (Hydrogen and Lithium) other than the code itself. They are still changing. >>> >>> On Fri, Jun 10, 2011 at 8:52 PM, Stephen Adams <[email protected]> wrote: >>>> >>>> >>>> On Fri, Jun 10, 2011 at 4:26 AM, Kevin Millikin <[email protected]> >>>> wrote: >>>> > Stephen, we also have this same constant folding in the >>>> > hydrogen->lithium >>>> > translation, with the drawback that it generates dead code for the >>>> > not-taken >>>> > branch and a jump around it on one side or the other. I'd be >>>> > interested in >>>> > knowing if it's this dead code and jump you want to get rid of, or if >>>> > we >>>> > have a case where that's really not working for us and we're testing >>>> > the >>>> > value of the constant. >>>> >>>> I wanted to get rid of the jump over the dead code. In the unoptimized >>>> version (I expect most code stays in that state) I see two unnecessary >>>> jumps >>>> and two unused embedded objects. >>>> I looked at a few scripts from a large app and there are hundreds of >>>> occurrences of !0 and !1. >>>> The one I looked at in detail, half of the occurrences of 0 and 1 were in >>>> !0 and !1. >>>> The dead code must exert some memory pressure and it contains embedded >>>> <true> or <false> objects, so there is more overhead than just the >>>> instruction bytes. >>>> >>>> Constant folding early should improve throughput at all levels of the >>>> system. I don't think constant folding here it is particularly evil, since >>>> the only sane reason to write !0 is as a shorthand for 'true'. >>>> Is there a document that would tell me what hydrogen and lithium are? >>>> From my previous email to Kasper: >>>> >>>> --- Raw source --- >>>> (n) { >>>> g = ! 0; >>>> } >>>> >>>> >>>> --- Code --- >>>> kind = FUNCTION >>>> name = testB0 >>>> Instructions (size = 76) >>>> 0xf2c02a20 0 55 push ebp >>>> 0xf2c02a21 1 89e5 mov ebp,esp >>>> 0xf2c02a23 3 56 push esi >>>> 0xf2c02a24 4 57 push edi >>>> 0xf2c02a25 5 3b2500a5930c cmp esp,[0xc93a500] >>>> 0xf2c02a2b 11 7305 jnc 18 (0xf2c02a32) >>>> 0xf2c02a2d 13 e88e5afeff call 0xf2be84c0 ;; debug: >>>> statement 157 >>>> ;; code: >>>> STUB, StackCheck, minor: 0 >>>> 0xf2c02a32 18 eb10 jmp 36 (0xf2c02a44) >>>> 0xf2c02a34 20 3da580c0f2 cmp eax, 0xf2c080a5 ;; object: >>>> 0xf2c080a5 <true> >>>> 0xf2c02a39 25 0f840c000000 jz 43 (0xf2c02a4b) >>>> 0xf2c02a3f 31 e900000000 jmp 36 (0xf2c02a44) >>>> 0xf2c02a44 36 b8a580c0f2 mov eax,0xf2c080a5 ;; object: >>>> 0xf2c080a5 <true> >>>> 0xf2c02a49 41 eb05 jmp 48 (0xf2c02a50) >>>> 0xf2c02a4b 43 b8c580c0f2 mov eax,0xf2c080c5 ;; object: >>>> 0xf2c080c5 <false> >>>> 0xf2c02a50 48 b951c3c0f2 mov ecx,0xf2c0c351 ;; object: >>>> 0xf2c0c351 <String[1]: g> >>>> 0xf2c02a55 53 8b5617 mov edx,[esi+23] >>>> 0xf2c02a58 56 e803b9feff call StoreIC_Initialize (0xf2bee360) >>>> ;; debug: statement 165 >>>> ;; debug: >>>> position 167 >>>> ;; code: >>>> contextual, STORE_IC, UNINITIALIZED >>>> 0xf2c02a5d 61 b85180c0f2 mov eax,0xf2c08051 ;; object: >>>> 0xf2c08051 <undefined> >>>> 0xf2c02a62 66 89ec mov esp,ebp ;; debug: >>>> position 174 >>>> ;; js return >>>> 0xf2c02a64 68 5d pop ebp >>>> 0xf2c02a65 69 c20800 ret 0x8 >>>> >>>> Stack checks (size = 0) >>>> ast_id pc_offset >>>> >>>> RelocInfo (size = 23) >>>> 0xf2c02a2d statement position (157) >>>> 0xf2c02a2e code target (STUB) (0xf2be84c0) >>>> 0xf2c02a35 embedded object (0xf2c080a5 <true>) >>>> 0xf2c02a45 embedded object (0xf2c080a5 <true>) >>>> 0xf2c02a4c embedded object (0xf2c080c5 <false>) >>>> 0xf2c02a51 embedded object (0xf2c0c351 <String[1]: g>) >>>> 0xf2c02a58 statement position (165) >>>> 0xf2c02a58 position (167) >>>> 0xf2c02a59 code target (context) (STORE_IC) (0xf2bee360) >>>> 0xf2c02a5e embedded object (0xf2c08051 <undefined>) >>>> 0xf2c02a62 position (174) >>>> 0xf2c02a62 js return >>>> >>>> >>>> --- Raw source --- >>>> (n) { >>>> g = ! 0; >>>> } >>>> >>>> >>>> --- Optimized code --- >>>> kind = OPTIMIZED_FUNCTION >>>> name = testB0 >>>> stack_slots = 0 >>>> Instructions (size = 85) >>>> 0xf2c70aa0 0 55 push ebp >>>> 0xf2c70aa1 1 89e5 mov ebp,esp >>>> 0xf2c70aa3 3 56 push esi >>>> 0xf2c70aa4 4 57 push edi >>>> 0xf2c70aa5 5 8b45fc mov eax,[ebp-4] >>>> 0xf2c70aa8 8 3b2500b5bb0c cmp esp,[0xcbbb500] >>>> 0xf2c70aae 14 7308 jnc 24 (0xf2c70ab8) >>>> 0xf2c70ab0 16 8b75fc mov esi,[ebp-4] >>>> 0xf2c70ab3 19 e8085afeff call 0xf2c564c0 ;; code: >>>> STUB, StackCheck, minor: 0 >>>> 0xf2c70ab8 24 e90b000000 jmp 40 (0xf2c70ac8) >>>> 0xf2c70abd 29 c7c0c560c7f2 mov eax,0xf2c760c5 ;; object: >>>> 0xf2c760c5 <false> >>>> 0xf2c70ac3 35 e906000000 jmp 46 (0xf2c70ace) >>>> 0xf2c70ac8 40 c7c0a560c7f2 mov eax,0xf2c760a5 ;; object: >>>> 0xf2c760a5 <true> >>>> 0xf2c70ace 46 8905c46fc1f2 mov [0xf2c16fc4],eax ;; global >>>> property cell >>>> 0xf2c70ad4 52 b85160c7f2 mov eax,0xf2c76051 ;; object: >>>> 0xf2c76051 <undefined> >>>> 0xf2c70ad9 57 89ec mov esp,ebp >>>> 0xf2c70adb 59 5d pop ebp >>>> 0xf2c70adc 60 c20800 ret 0x8 >>>> 0xf2c70adf 63 90 nop >>>> 0xf2c70ae0 64 90 nop >>>> 0xf2c70ae1 65 90 nop >>>> 0xf2c70ae2 66 90 nop >>>> 0xf2c70ae3 67 90 nop >>>> >>>> Safepoints (size = 17) >>>> 0xf2c70ab8 24 11111111 (sp -> fp) 0 >>>> >>>> RelocInfo (size = 6) >>>> 0xf2c70ab4 code target (STUB) (0xf2c564c0) >>>> 0xf2c70abf embedded object (0xf2c760c5 <false>) >>>> 0xf2c70aca embedded object (0xf2c760a5 <true>) >>>> 0xf2c70ad0 global property cell >>>> 0xf2c70ad5 embedded object (0xf2c76051 <undefined>) >>>> >>>> > >>>> > On Fri, Jun 10, 2011 at 12:05 PM, <[email protected]> wrote: >>>> >> >>>> >> http://codereview.chromium.org/7044100/diff/1/src/parser.cc >>>> >> File src/parser.cc (right): >>>> >> >>>> >> >>>> >> http://codereview.chromium.org/7044100/diff/1/src/parser.cc#newcode2559 >>>> >> src/parser.cc:2559: if (value == 0) { >>>> >> On 2011/06/10 09:20:08, Kasper Lund wrote: >>>> >>> >>>> >>> Could this be something ala: >>>> >> >>>> >>> Handle<Object> boolean(isolate->heap()->ToBoolean(value == 0)); >>>> >>> return new(zone()) Literal(boolean); >>>> >> >>>> >>> or perhaps consider negating the result of calling >>>> >>> expression->AsLiteral()->handle()->ToBoolean() and refactoring the >>>> >> >>>> >> code so it >>>> >>> >>>> >>> works for cases where the expression is a non-numeric literal. >>>> >> >>>> >> We already have this constant folding in void >>>> >> FullCodeGenerator::TestContext::Plug(Handle<Object> lit). It just >>>> >> needs >>>> >> to be 'ported' to TestContext::BuildBranch(HValue* value) in >>>> >> hydrogen.cc. >>>> >> >>>> >> I would prefer that solution rather than having it here, because it >>>> >> can >>>> >> properly cope with things like >>>> >> >>>> >> x = 0 >>>> >> if (!x) ... >>>> >> >>>> >> http://codereview.chromium.org/7044100/ >>>> > >>>> > >>>> >>> >> >> > -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
