| Summary: | DFG should be able to print disassembly interleaved with the IR | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Filip Pizlo <fpizlo> | ||||||
| Component: | JavaScriptCore | Assignee: | Nobody <webkit-unassigned> | ||||||
| Status: | RESOLVED FIXED | ||||||||
| Severity: | Normal | CC: | barraclough, fpizlo, ggaren, oliver, rakuco, webkit.review.bot | ||||||
| Priority: | P2 | ||||||||
| Version: | 528+ (Nightly build) | ||||||||
| Hardware: | All | ||||||||
| OS: | All | ||||||||
| Attachments: |
|
||||||||
|
Description
Filip Pizlo
2012-06-20 00:07:21 PDT
Here's a sample:
Generated JIT code for DFG CodeBlock 0x109905400:
Code at [0x3ab1c260d960, 0x3ab1c260db2e):
0x3ab1c260d960: pop %rcx
0x3ab1c260d961: mov %rcx, -0x10(%r13)
0x3ab1c260d965: mov 0x109905400, %r11
0x3ab1c260d96f: mov %r11, -0x8(%r13)
0x3ab1c260d973: lea 0x18(%r13), %rdx
0x3ab1c260d977: mov 0x10986cea0, %r11
0x3ab1c260d981: cmp %rdx, (%r11)
0x3ab1c260d984: jb 0x3ab1c260da2d
0x3ab1c260d98a: mov 0x109905e54, %r11
0x3ab1c260d994: add 0x1, (%r11)
Block #0 (bc#0):
Predecessors:
Phi Nodes:
0: < 3:-> SetArgument(, arg0(F<Final>)) predicting Final, double ratio 0.000000
1: < 3:1> GetLocal(@0, JS, arg0(F<Final>)) predicting Final, double ratio 0.000000
0x3ab1c260d998: mov -0x38(%r13), %rax
2: <!0:-> CheckStructure(@1<Final>, MustGen|CanExit, struct(0x10a2b7c00))
0x3ab1c260d99c: test %rax, %r15
0x3ab1c260d99f: jnz 0x3ab1c260da94
0x3ab1c260d9a5: mov 0x10a2b7c00, %r11
0x3ab1c260d9af: cmp %r11, 0x8(%rax)
0x3ab1c260d9b3: jnz 0x3ab1c260daaa
4: < 1:1> GetByOffset(@1<Final>, JS, id0{direction}, 7) predicting Int
0x3ab1c260d9b9: mov 0x38(%rax), %rax
6: <!2:2> GetGlobalVar(JS|MustGen, global20(0x1098bcf38)) predicting Final
0x3ab1c260d9bd: mov 0x1098bcf38, %rdx
0x3ab1c260d9c7: mov (%rdx), %rdx
8: <!0:-> CheckStructure(@6<Final>, MustGen|CanExit, struct(0x10a2b9960))
0x3ab1c260d9ca: test %rdx, %r15
0x3ab1c260d9cd: jnz 0x3ab1c260dac0
0x3ab1c260d9d3: mov 0x10a2b9960, %r11
0x3ab1c260d9dd: cmp %r11, 0x8(%rdx)
0x3ab1c260d9e1: jnz 0x3ab1c260dad6
9: < 1:2> GetByOffset(@6<Final>, JS, id1{FORWARD}, 5) predicting Int
0x3ab1c260d9e7: mov 0x28(%rdx), %rdx
11: <!1:2> CompareEq(@4<Int32>, @9<Int32>, Boolean|MustGen|MightClobber|CanExit)
0x3ab1c260d9eb: cmp %r14, %rax
0x3ab1c260d9ee: jb 0x3ab1c260daec
0x3ab1c260d9f4: cmp %r14, %rdx
0x3ab1c260d9f7: jb 0x3ab1c260db02
0x3ab1c260d9fd: cmp %edx, %eax
0x3ab1c260d9ff: jnz 0x3ab1c260da16
Block #1 (bc#33):
Predecessors: #0
Phi Nodes: @14->(@0)
15: < 2:2> GetLocal(@14, JS, arg0(F<Final>)) predicting Final, double ratio 0.000000
0x3ab1c260da05: mov -0x38(%r13), %rcx
16: <!0:-> CheckStructure(@15<Final>, MustGen, struct(0x10a2b7c00))
17: < 1:2> GetByOffset(@15<Final>, JS, id2{v2}, 6) predicting Final
0x3ab1c260da09: mov 0x30(%rcx), %rcx
18: < 1:-> SetLocal(@17<Final>, , r0(J<Final>)) predicting Final, double ratio 0.000000
0x3ab1c260da0d: mov %rcx, 0x0(%r13)
19: <!0:-> Jump(MustGen, T:#3)
0x3ab1c260da11: jmp 0x3ab1c260da1f
Block #2 (bc#44):
Predecessors: #0
Phi Nodes: @20->(@0)
21: < 1:2> GetLocal(@20, JS, arg0(F<Final>)) predicting Final, double ratio 0.000000
0x3ab1c260da16: mov -0x38(%r13), %rbx
22: <!1:2> GetById(@21<Final>, JS|MustGen|Clobbers|CanExit, id3{v1}) predicting None
0x3ab1c260da1a: jmp 0x3ab1c260db18
Block #3 (bc#53):
Predecessors: #2 #1
Phi Nodes: @25->(@23, @18)
26: < 1:2> GetLocal(@25, JS, r0(J<Final>)) predicting Final, double ratio 0.000000
0x3ab1c260da1f: mov 0x0(%r13), %rax
27: <!0:-> Return(@26<Final>, MustGen)
0x3ab1c260da23: mov -0x10(%r13), %rdx
0x3ab1c260da27: mov -0x28(%r13), %r13
0x3ab1c260da2b: push %rdx
0x3ab1c260da2c: ret
(End Of Main Path)
0x3ab1c260da2d: mov %rsp, %rdi
0x3ab1c260da30: mov %r13, 0x58(%rsp)
0x3ab1c260da35: mov 0x0, -0x2c(%r13)
0x3ab1c260da3d: mov 0x10943e4c0, %r11
0x3ab1c260da47: call %r11
0x3ab1c260da4a: jmp 0x3ab1c260d98a
0x3ab1c260da4f: pop %rcx
0x3ab1c260da50: mov %rcx, -0x10(%r13)
0x3ab1c260da54: mov 0x109905400, %r11
0x3ab1c260da5e: mov %r11, -0x8(%r13)
0x3ab1c260da62: mov -0x30(%r13), %edx
0x3ab1c260da66: cmp 0x1, %edx
0x3ab1c260da69: jae 0x3ab1c260d973
0x3ab1c260da6f: mov %rsp, %rdi
0x3ab1c260da72: mov %r13, 0x58(%rsp)
0x3ab1c260da77: mov 0x1, -0x2c(%r13)
0x3ab1c260da7f: mov 0x109440650, %r11
0x3ab1c260da89: call %r11
0x3ab1c260da8c: mov %rax, %r13
0x3ab1c260da8f: jmp 0x3ab1c260d973
0x3ab1c260da94: mov 0x109859828, %r11
0x3ab1c260da9e: mov 0x0, (%r11)
0x3ab1c260daa5: jmp 0x3ab1c260db40
0x3ab1c260daaa: mov 0x109859828, %r11
0x3ab1c260dab4: mov 0x1, (%r11)
0x3ab1c260dabb: jmp 0x3ab1c260db40
0x3ab1c260dac0: mov 0x109859828, %r11
0x3ab1c260daca: mov 0x2, (%r11)
0x3ab1c260dad1: jmp 0x3ab1c260db40
0x3ab1c260dad6: mov 0x109859828, %r11
0x3ab1c260dae0: mov 0x3, (%r11)
0x3ab1c260dae7: jmp 0x3ab1c260db40
0x3ab1c260daec: mov 0x109859828, %r11
0x3ab1c260daf6: mov 0x4, (%r11)
0x3ab1c260dafd: jmp 0x3ab1c260db40
0x3ab1c260db02: mov 0x109859828, %r11
0x3ab1c260db0c: mov 0x5, (%r11)
0x3ab1c260db13: jmp 0x3ab1c260db40
0x3ab1c260db18: mov 0x109859828, %r11
0x3ab1c260db22: mov 0x6, (%r11)
0x3ab1c260db29: jmp 0x3ab1c260db40
Created attachment 148517 [details]
the patch
Attachment 148517 [details] did not pass style-queue:
Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/CMakeLists.txt', u'S..." exit_code: 1
Source/JavaScriptCore/dfg/DFGGraph.h:185: The parameter name "node" adds no information, so it should be removed. [readability/parameter_name] [5]
Source/JavaScriptCore/dfg/DFGGraph.h:186: The parameter name "node" adds no information, so it should be removed. [readability/parameter_name] [5]
Total errors found: 2 in 21 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 148519 [details]
improved patch
Fixed style issues.
Also fixed a small issue in dumping of code that was not generated. The previous patch would dump neither the IR nor the disassembly for such nodes. The current patch will dump the IR but not the disassembly, and will make sure that the disassembly for the previous node is dumped correctly. To be clear, what I mean by "code that was not generated" is either code that was after a forced OSR exit (the IR for such code is relevant from the standpoint of liveness) or Branches that were not really emitted due to peephole optimizations.
Comment on attachment 148519 [details] improved patch View in context: https://bugs.webkit.org/attachment.cgi?id=148519&action=review r=me, but please double-check dominators removal, and add a comment to the ChangeLog explaining why. > Source/JavaScriptCore/dfg/DFGDriver.cpp:-95 > - dfg.m_dominators.compute(dfg); Was this on purpose? Seems wrong. (In reply to comment #5) > (From update of attachment 148519 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=148519&action=review > > r=me, but please double-check dominators removal, and add a comment to the ChangeLog explaining why. > > > Source/JavaScriptCore/dfg/DFGDriver.cpp:-95 > > - dfg.m_dominators.compute(dfg); > > Was this on purpose? Seems wrong. It was, but then I forgot to finish the job. We have a couple of things in the DFG code-base that were done as first steps to SSA but that we never used. They should either get killed or find new life. One is the redundant Phi elimination phase, which is just now wrong. The code is sitting there but isn't called from anywhere. The other is dominators. I thought I'd use them for arguments simplification, but then I decided not to. But, in the meantime, I found dominators to be useful for IR dumps. They make it easier for me to figure out block relationships. So, what this patch was supposed to do, is remove unconditional dominator computation, and instead call Dominators::computeIfNecessary() inside of Disassembler. I'll land with that change. Landed in http://trac.webkit.org/changeset/120834 Forgot to add the ChangeLog comment, added in http://trac.webkit.org/changeset/120836 |