<div dir="ltr">Hey Eliot,<div><br></div><div>thanks for helping.</div><div class="gmail_extra"><br><div class="gmail_quote">2015-04-29 2:25 GMT+02:00 Eliot Miranda <span dir="ltr"><<a href="mailto:eliot.miranda@gmail.com" target="_blank">eliot.miranda@gmail.com</a>></span>:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr">Hi Clément,<div><br></div><div> the issue with the assert fail on starting the current Sista is one of incorrect pc mapping. The only difference between SistaStackToRegisterMappingCogit and StackToRegisterMappingCogit is that in the Sista cogit #== is mapped, see</div><div><br></div><div>SistaStackToRegisterMappingCogit class>>generatorTableFrom: anArray</div><div><span style="white-space:pre-wrap"> </span>"Override to replace the unmapped, non-counting inlined #== with a mapped counting inlined #==."</div><div><span style="white-space:pre-wrap"> </span>| table |</div><div><span style="white-space:pre-wrap"> </span>table := super generatorTableFrom: anArray.</div><div><span style="white-space:pre-wrap"> </span>table object do:</div><div><span style="white-space:pre-wrap"> </span>[:descriptor|</div><div><span style="white-space:pre-wrap"> </span> descriptor generator == #genSpecialSelectorEqualsEquals ifTrue:</div><div><span style="white-space:pre-wrap"> </span>[descriptor</div><div><span style="white-space:pre-wrap"> </span>isMapped: true;</div><div><span style="white-space:pre-wrap"> </span>isMappedInBlock: true;</div><div><span style="white-space:pre-wrap"> </span>needsFrameFunction: nil]].</div><div><span style="white-space:pre-wrap"> </span>^table</div></div></blockquote><div><br></div><div>Ok I'll fix that </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr"><div><br></div><div>Loking at what you've done I guess you've removed the need to map #== mapped, in which case the above should be deleted and #== unmapped. But looking at the generated code for #== followed by a jump I can't understand how the code works.</div><div><br></div><div>What I see is counting instructions followed by the #== comparison, as in</div><div><br></div><div><div>SistaStackToRegisterMappingCogit</div><div><span style="white-space:pre-wrap"> </span>genAndDis: IdentityDictionary>>#scanFor:</div><div><span style="white-space:pre-wrap"> </span>options: #(<span style="white-space:pre-wrap"> </span>SistaVM true</div><div><span style="white-space:pre-wrap"> </span>ObjectMemory Spur32BitCoMemoryManager</div><div><span style="white-space:pre-wrap"> </span>MULTIPLEBYTECODESETS true</div><div><span style="white-space:pre-wrap"> </span>ISA IA32</div><div><span style="white-space:pre-wrap"> </span>bytecodeTableInitializer initializeBytecodeTableForSqueakV3PlusClosuresSistaV1Hybrid)</div></div><div><br></div><div>which generates the following for the "(element := array at: index) == nil" #== comparison:</div><div><br></div><div><div>000014c6: movl $0x00100098=#at:, %ecx : B9 98 00 10 00 </div><div>000014cb: call .+0xffffefa0 (0x00000470=ceSend1Args) : E8 A0 EF FF FF </div><div>IsSendCall bc 49/50:</div><div>000014d0: movl %edx, -24(%ebp) : 89 55 E8 </div><div><br></div><div>do the "executed jump" count for the following jump:</div><div>000014d3: movl %ds:0x40090, %edi : 8B 3D 90 00 04 00 </div><div>000014d9: subl $0x00010000, %edi : 81 EF 00 00 01 00 </div><div>"If the count trips, jump to a second occurrence of the comparison code"</div><div>000014df: jb .+0x00000035 (0x00001516) : 72 35 </div><div>"write back modified count"</div><div>000014e1: movl %edi, %ds:0x40090 : 89 3D 90 00 04 00 </div><div><br></div><div>"do the #== nil comparison"</div><div>000014e7: cmpl $0x00100000=nil, %edx : 81 FA 00 00 10 00 </div><div>"jump to the #== nil being true continuation"</div><div>000014ed: jz .+0x00000086 (0x00001579) : 0F 84 86 00 00 00 </div><div>"fall throguh to unforwarding code given that the #== nil could be false because of forwarding, and it should be retried"</div><div>000014f3: movl %edx, %eax : 89 D0 </div><div>000014f5: andl $0x00000003, %eax : 83 E0 03 </div><div>000014f8: jnz .+0x0000000e (0x00001508) : 75 0E </div></div><div><br></div><div>What I don't understand is if you trip the count how is the #== ever executed? Don't you just continue assuming the result was either true or false? That can't be right. So I'm confused and hence I haven't deleted the generatorTableFrom: method. I don't understand the code and so shouldn't mess with it :)</div></div></blockquote><div><br></div><div>Ok I think you understood it at some point and you forgot :-).</div><div><br></div><div>Basically #== tries to use the inlined version merged with the following branch, but if the counter trips, #== is executed in an inlined version that ignores the branch, pushing true or false on the simulated stack, and then the code generated for the branch uses the result of #== pushed on the simulated stack, which calls the trampoline for the tripping counter.</div><div><br></div><div>Conceptually, this is the code generated for #==, if followed by branch true, for this example:</div><div><br></div><div><div>foo</div><div><span class="" style="white-space:pre"> </span>^ instVar1 == instVar2 ifTrue: [ 1 ] ifFalse: [ 2 ]</div></div><div><br></div><div><i>Loading the operands</i></div><div><div>0000146a: movl -12(%ebp), %edx : 8B 55 F4 </div><div>0000146d: movl %ds:0xc(%edx), %edi : 8B 7A 0C </div><div>00001470: movl %ds:0x8(%edx), %esi : 8B 72 08 </div><div>%edi <- instVar1</div><div>%esi <- instVar2</div><div><br></div><div><i>counter logic (execution count)</i></div><div>00001473: movl %ds:0x40090, %ebx : 8B 1D 90 00 04 00 </div><div>%ebx <- counter value for this branch</div><div>00001479: subl $0x00010000, %ebx : 81 EB 00 00 01 00 </div><div>execution count - 1</div><div>0000147f: jb .+0x00000046 (<font color="#9900ff">0x000014c7</font>) : 72 46 </div><div>if trip, jumps to the alternative #== which push true or false on stack for the branch code</div><div>00001481: movl %ebx, %ds:0x40090 : 89 1D 90 00 04 00 </div><div>write back the counter to the memory location</div><div><br></div><div><i>#== followed by inlined branch version</i></div><div>00001487: cmpl %edi, %esi : 39 FE </div><div>00001489: jz .+0x000000a6 (<font color="#6aa84f">0x00001535</font>) : 0F 84 A6 00 00 00 </div><div>compare instVar1 and instVar2 and jump if equals to the branch that pushes 1</div><div><font size="1">0000148f: movl %edi, %eax : 89 F8 </font></div><div><font size="1">00001491: andl $0x00000003, %eax : 83 E0 03 </font></div><div><font size="1">00001494: jnz .+0x0000000e (0x000014a4) : 75 0E </font></div><div><font size="1">00001496: movl %ds:(%edi), %eax : 8B 07 </font></div><div><font size="1">00001498: andl $0x003ffff7, %eax : 25 F7 FF 3F 00 </font></div><div><font size="1">0000149d: jnz .+0x00000005 (0x000014a4) : 75 05 </font></div><div><font size="1">0000149f: movl %ds:0x8(%edi), %edi : 8B 7F 08 </font></div><div><font size="1">000014a2: jmp .+0xffffffe3 (0x00001487) : EB E3 </font></div><div><font size="1">000014a4: movl %esi, %eax : 89 F0 </font></div><div><font size="1">000014a6: andl $0x00000003, %eax : 83 E0 03 </font></div><div><font size="1">000014a9: jnz .+0x0000000e (0x000014b9) : 75 0E </font></div><div><font size="1">000014ab: movl %ds:(%esi), %eax : 8B 06 </font></div><div><font size="1">000014ad: andl $0x003ffff7, %eax : 25 F7 FF 3F 00 </font></div><div><font size="1">000014b2: jnz .+0x00000005 (0x000014b9) : 75 05 </font></div><div><font size="1">000014b4: movl %ds:0x8(%esi), %esi : 8B 76 08 </font></div><div><font size="1">000014b7: jmp .+0xffffffce (0x00001487) : EB CE </font></div><div>forwarder checks<br></div><div><br></div><div><i>counter logic (branch count)</i></div><div>000014b9: subl $0x00000001, %ebx : 83 EB 01 </div><div>branch count - 1</div><div>000014bc: movl %ebx, %ds:0x40090 : 89 1D 90 00 04 00 </div><div>write back the counter to the memory location<br></div><div><br></div><div>000014c2: jmp .+0x00000075 (<font color="#ff0000">0x0000153c</font>) : E9 75 00 00 00 </div><div>The result of #== was false let's jump to the branch that pushes 2.</div><div><br></div><div><i>We arrive here only if the counter has tripped</i></div><div><font color="#9900ff">000014c7</font>: cmpl %edi, %esi : 39 FE </div><div>000014c9: jz .+0x00000031 (0x000014fc) : 74 31 </div><div><font size="1">000014cb: movl %edi, %eax : 89 F8 </font></div><div><font size="1">000014cd: andl $0x00000003, %eax : 83 E0 03 </font></div><div><font size="1">000014d0: jnz .+0x0000000e (0x000014e0) : 75 0E </font></div><div><font size="1">000014d2: movl %ds:(%edi), %eax : 8B 07 </font></div><div><font size="1">000014d4: andl $0x003ffff7, %eax : 25 F7 FF 3F 00 </font></div><div><font size="1">000014d9: jnz .+0x00000005 (0x000014e0) : 75 05 </font></div><div><font size="1">000014db: movl %ds:0x8(%edi), %edi : 8B 7F 08 </font></div><div><font size="1">000014de: jmp .+0xffffffe7 (0x000014c7) : EB E7 </font></div><div><font size="1">000014e0: movl %esi, %eax : 89 F0 </font></div><div><font size="1">000014e2: andl $0x00000003, %eax : 83 E0 03 </font></div><div><font size="1">000014e5: jnz .+0x0000000e (0x000014f5) : 75 0E </font></div><div><font size="1">000014e7: movl %ds:(%esi), %eax : 8B 06 </font></div><div><font size="1">000014e9: andl $0x003ffff7, %eax : 25 F7 FF 3F 00 </font></div><div><font size="1">000014ee: jnz .+0x00000005 (0x000014f5) : 75 05 </font></div><div><font size="1">000014f0: movl %ds:0x8(%esi), %esi : 8B 76 08 </font></div><div><font size="1">000014f3: jmp .+0xffffffd2 (0x000014c7) : EB D2 </font></div><div>forwarder checks</div><div>000014f5: movl $0x00100008=false, %esi : BE 08 00 10 00 </div><div>000014fa: jmp .+0x00000005 (0x00001501) : EB 05 </div><div>000014fc: movl $0x00100010=true, %esi : BE 10 00 10 00 </div><div>Inlined version of #== that answers true or false on the simulated stack.</div><div><br></div><div><i>Now we're in the branch logic (genJumpIf), not the #== logic (genSpecialSelectorEqualsEquals)</i></div><div>00001501: movl %esi, %eax : 89 F0 </div><div>Here we get true or false, or some other result if this instruction is a fixup.</div><div>00001503: movl %ds:0x40090, %edi : 8B 3D 90 00 04 00 </div><div>00001509: subl $0x00010000, %edi : 81 EF 00 00 01 00 </div><div>0000150f: jb .+0x0000001d (0x0000152e) : 72 1D </div><div>00001511: movl %edi, %ds:0x40090 : 89 3D 90 00 04 00 </div><div>Here is the counter logic again, if we had tripped in #==, we trip again.</div><div>00001517: subl $0x00100008=false, %eax : 2D 08 00 10 00 </div><div>0000151c: jz .+0x0000001e (<font color="#ff0000">0x0000153c</font>) : 74 1E </div><div>0000151e: subl $0x00000001, %edi : 83 EF 01 </div><div>00001521: movl %edi, %ds:0x40090 : 89 3D 90 00 04 00 </div><div>00001527: cmpl $0x00000008, %eax : 83 F8 08 </div><div>0000152a: jz .+0x00000009 (<font color="#6aa84f">0x00001535</font>) : 74 09 </div><div>0000152c: xorl %edi, %edi : 31 FF </div><div>0000152e: call .+0xfffff9e5 (0x00000f18=ceSendMustBeBooleanAddFalseTrampoline) : E8 E5 F9 FF FF </div><div>HasBytecodePC bc 19/20:</div><div>This is the trampoline for mustBeBoolean and counterTrip.</div><div>00001533: jmp .+0xffffffce (0x00001503) : EB CE </div><div>After a tripping counter, we assume the counters are reset and we jump back to the comparison. We have to jump back to the execution count counter logic not to confuse the branch counter logic</div><div><br></div><div><i>Now this is the push 1, jump and push 2 logic</i></div><div><font color="#6aa84f">00001535</font>: pushl $0x00000003 : 68 03 00 00 00 </div><div>0000153a: jmp .+0x00000005 (0x00001541) : EB 05 </div><div><font color="#ff0000">0000153c</font>: pushl $0x00000005 : 68 05 00 00 00 </div><div>Here the two branches push 1 and 2 on stack</div><div><br></div><div>00001541: popl %edx : 5A </div><div>00001542: movl %ebp, %esp : 89 EC </div><div>00001544: popl %ebp : 5D </div><div>00001545: ret $0x0004 : C2 04 00 </div></div><div>Return</div><div><br></div><div>Everything looks correct to me. The case you showed includes a and: so there's the additional fixup, I can't explain here because the code is twice longer and hard to follow, but I believe it is also correct.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div class="gmail_extra"><div><div class="h5"><br><div class="gmail_quote">On Tue, Apr 28, 2015 at 5:12 PM, <span dir="ltr"><<a href="mailto:commits@source.squeak.org" target="_blank">commits@source.squeak.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><br>
Eliot Miranda uploaded a new version of VMMaker to project VM Maker:<br>
<a href="http://source.squeak.org/VMMaker/VMMaker.oscog-eem.1270.mcz" target="_blank">http://source.squeak.org/VMMaker/VMMaker.oscog-eem.1270.mcz</a><br>
<br>
==================== Summary ====================<br>
<br>
Name: VMMaker.oscog-eem.1270<br>
Author: eem<br>
Time: 28 April 2015, 5:11:29.049 pm<br>
UUID: a6737c7e-fb6b-4fe9-a44e-6b01396cff34<br>
Ancestors: VMMaker.oscog-eem.1269<br>
<br>
Nuke obsolete printMcpc:Bcpc:on:<br>
<br>
Correct SistaStackToRegisterMappingCogit>>genJumpIf:to:<br>
to reload counterReg instead of saving and<br>
restoring it around counter callback trampoline.<br>
<br>
=============== Diff against VMMaker.oscog-eem.1269 ===============<br>
<br>
Item was removed:<br>
- ----- Method: Cogit>>printMcpc:Bcpc:on: (in category 'method map') -----<br>
- printMcpc: mcpc Bcpc: bcpc on: aStream<br>
- <doNotGenerate><br>
- aStream ensureCr.<br>
- mcpc printOn: aStream base: 16.<br>
- aStream space; tab; print: bcpc; cr; flush.<br>
- ^0!<br>
<br>
Item was changed:<br>
----- Method: SistaStackToRegisterMappingCogit>>genJumpIf:to: (in category 'bytecode generator support') -----<br>
genJumpIf: boolean to: targetBytecodePC<br>
"The heart of performance counting in Sista. Conditional branches are 6 times less<br>
frequent than sends and can provide basic block frequencies (send counters can't).<br>
Each conditional has a 32-bit counter split into an upper 16 bits counting executions<br>
and a lower half counting untaken executions of the branch. Executing the branch<br>
decrements the upper half, tripping if the count goes negative. Not taking the branch<br>
decrements the lower half. N.B. We *do not* eliminate dead branches (true ifTrue:/true ifFalse:)<br>
so that scanning for send and branch data is simplified and that branch data is correct."<br>
<inline: false><br>
| desc ok counterAddress countTripped retry counterReg |<br>
<var: #ok type: #'AbstractInstruction *'><br>
<var: #desc type: #'CogSimStackEntry *'><br>
<var: #retry type: #'AbstractInstruction *'><br>
<var: #countTripped type: #'AbstractInstruction *'><br>
<br>
(coInterpreter isOptimizedMethod: methodObj) ifTrue: [ ^ super genJumpIf: boolean to: targetBytecodePC ].<br>
<br>
self ssFlushTo: simStackPtr - 1.<br>
desc := self ssTop.<br>
self ssPop: 1.<br>
desc popToReg: TempReg.<br>
+<br>
-<br>
"We prefer calleeSaved to avoid saving it across the trap trip trampoline"<br>
counterReg := self allocateRegPreferringCalleeSavedNotConflictingWith: 0.<br>
retry := self Label.<br>
self<br>
genExecutionCountLogicInto: [ :cAddress :countTripBranch |<br>
counterAddress := cAddress.<br>
countTripped := countTripBranch ]<br>
counterReg: counterReg.<br>
counterIndex := counterIndex + 1.<br>
+<br>
-<br>
"Cunning trick by LPD. If true and false are contiguous subtract the smaller.<br>
Correct result is either 0 or the distance between them. If result is not 0 or<br>
their distance send mustBeBoolean."<br>
self assert: (objectMemory objectAfter: objectMemory falseObject) = objectMemory trueObject.<br>
self annotate: (self SubCw: boolean R: TempReg) objRef: boolean.<br>
self JumpZero: (self ensureFixupAt: targetBytecodePC - initialPC).<br>
<br>
self genFallsThroughCountLogicCounterReg: counterReg counterAddress: counterAddress.<br>
<br>
self CmpCq: (boolean == objectMemory falseObject<br>
ifTrue: [objectMemory trueObject - objectMemory falseObject]<br>
ifFalse: [objectMemory falseObject - objectMemory trueObject])<br>
R: TempReg.<br>
ok := self JumpZero: 0.<br>
self MoveCq: 0 R: counterReg. "if counterReg is 0 this is a mustBeBoolean, not a counter trip."<br>
+<br>
+ self flag: 'Hi Clément. You can''t just save things to the Smalltalk stack. You can /only/ save things that execution expects to be there on a context''s stack, because this frame may get mapped to a context object and then back, and gc''ed etc. The counter reg does not contain an object so is a complete no-no on the Smalltalk stack. On the C stack in the trampoline is OK, not on the Smalltalk stack in method execution. So instead of saving and restoring the counterReg around the call, something we can''t do, we can reload it after the call'.<br>
+ false ifTrue:<br>
+ ["If counterReg is caller saved then save it"<br>
+ (self register: counterReg isInMask: callerSavedRegMask) ifTrue: [ self PushR: counterReg ]].<br>
+<br>
-<br>
- "If counterReg is caller saved then save it"<br>
- (self register: counterReg isInMask: callerSavedRegMask) ifTrue: [ self PushR: counterReg ].<br>
-<br>
countTripped jmpTarget:<br>
(self CallRT: (boolean == objectMemory falseObject<br>
ifTrue: [ceSendMustBeBooleanAddFalseTrampoline]<br>
ifFalse: [ceSendMustBeBooleanAddTrueTrampoline])).<br>
<br>
"If we're in an image which hasn't got the Sista code loaded then the ceCounterTripped:<br>
trampoline will return directly to machine code, returning the boolean. So the code should<br>
jump back to the retry point. The trampoline makes sure that TempReg has been reloaded."<br>
self annotateBytecode: self Label.<br>
+<br>
+ self flag: 'see above'.<br>
+ false ifTrue:<br>
+ ["If counterReg is caller saved then restore it"<br>
+ (self register: counterReg isInMask: callerSavedRegMask) ifTrue: [ self PopR: counterReg ]].<br>
+<br>
+ "Note we /can't/ save and restore the counterReg's contents around the call since the stack can<br>
+ only contain what an interpreted context's stack would contain at the corresponding point. The<br>
+ counter is not an object, so can't be written to the stack. Hence we reload it after the call."<br>
+ self MoveAw: counterAddress R: counterReg.<br>
+<br>
-<br>
- "If counterReg is caller saved then restore it"<br>
- (self register: counterReg isInMask: callerSavedRegMask) ifTrue: [ self PopR: counterReg ].<br>
-<br>
self Jump: retry.<br>
ok jmpTarget: self Label.<br>
^0!<br>
<br>
Item was removed:<br>
- ----- Method: SistaStackToRegisterMappingCogit>>printMcpc:Bcpc:on: (in category 'method map') -----<br>
- printMcpc: mcpc Bcpc: bcpc on: aStream<br>
- <doNotGenerate><br>
- self shouldNotImplement!<br>
<br>
</blockquote></div><br><br clear="all"><div><br></div></div></div><span class=""><font color="#888888">-- <br><div>best,<div>Eliot</div></div>
</font></span></div>
</blockquote></div><br></div></div>