I'll help you land it on Monday, Stephen. Thanks a lot for the patch! Cheers, Kasper
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
