<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">2016-01-20 2:35 GMT+01: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><br></div><div><br></div><div> alas the bytecode <-> machine code mapping machinery isn't capable of distinguishing different variants of a complex bytecode such as extendedStoreAndPopBytecode. The map machinery is based on parsing bytecodes by looking up their descriptors. So if a bytecode descriptor is marked as mapped, all instantiations of the bytecode in machine code must be mapped, not just the ones we know we're going to need a map entry for for message handling etc. </div></div></blockquote><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>So where you write</div><div><br></div><div><span class=""><div>extendedStoreAndPopBytecode</div><div><span style="white-space:pre-wrap">        </span>| variableType variableIndex |</div><div><span style="white-space:pre-wrap">        </span>variableType := byte1 >> 6 bitAnd: 3.</div><div><span style="white-space:pre-wrap">        </span>variableIndex := byte1 bitAnd: 63.</div><div><span style="white-space:pre-wrap">        </span>variableType = 0 ifTrue:</div><div><span style="white-space:pre-wrap">                </span>[^self genStorePop: true ReceiverVariable: variableIndex].</div><div><span style="white-space:pre-wrap">        </span>variableType = 1 ifTrue:</div></span><span class=""><div><span style="white-space:pre-wrap">                </span>[self genStorePop: true TemporaryVariable: variableIndex.</div></span><span class=""><div><span style="white-space:pre-wrap">                </span>"needs a fake map entry is Immutability is ON..."</div></span><span class=""><div><span style="white-space:pre-wrap">                </span>self cppIf: IMMUTABILITY ifTrue: [ self annotateBytecode: self Label. ].</div></span><div><span style="white-space:pre-wrap">                </span>^ 0].</div><span class=""><div><span style="white-space:pre-wrap">        </span>variableType = 3 ifTrue:</div><div><span style="white-space:pre-wrap">                </span>[^self genStorePop: true LiteralVariable: variableIndex].</div><div><span style="white-space:pre-wrap">        </span>^EncounteredUnknownBytecode</div></span></div><div><br></div><div>you're actually going to have to write something like</div><div><br></div><div><span class=""><div>extendedStoreAndPopBytecode</div><div><span style="white-space:pre-wrap">        </span>| variableType variableIndex |</div><div><span style="white-space:pre-wrap">        </span>variableType := byte1 >> 6 bitAnd: 3.</div><div><span style="white-space:pre-wrap">        </span>variableIndex := byte1 bitAnd: 63.</div><div><span style="white-space:pre-wrap">        </span>variableType = 0 ifTrue:</div></span><div><span style="white-space:pre-wrap">                </span>[self genStorePop: true ReceiverVariable: variableIndex].</div><div><span style="white-space:pre-wrap">        </span>variableType = 1 ifTrue:</div><div><span style="white-space:pre-wrap">                </span>[self genStorePop: true TemporaryVariable: variableIndex].</div><div><span style="white-space:pre-wrap">        </span>variableType = 2 ifTrue:</div><div><span style="white-space:pre-wrap">                </span>[^EncounteredUnknownBytecode].</div><div><span style="white-space:pre-wrap">        </span>variableType = 3 ifTrue:</div><div><span style="white-space:pre-wrap">                </span>[self genStorePop: true LiteralVariable: variableIndex].<br><span style="white-space:pre-wrap">        "If </span>IMMUTABILITY<span style="white-space:pre-wrap"> is on then this bytecode could send back and so must be mapped."</span><br></div><div><span style="white-space:pre-wrap">        </span>self cppIf: IMMUTABILITY ifTrue: [self annotateBytecode: self lastOpcode]</div></div><div> <br></div></div></blockquote><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></div><div>I think this should fix your instabilities. What's happening is that your nearly-correct code creates the occasional method with a bad map and if the map is used (which it isn't often, but is in the debugger) then method execution can get screwed up.</div><div><br></div></div></blockquote><div><br class="">As stated in the commit comment, my code generates a correct map for all methods but the ones with primitive with error code... The test #testPCMappingFor:options: proves it (you can run it on all methods to check).</div><div><br></div><div>I don't think your change is a good idea because the mcpc mapped would be incorrect.</div><div><br></div><div>However, if you have an idea on how to handle correctly primitive with error code (the extStore for error code is compiled differently and lacks the map entry), it would be helpful. I tried to generate an entry at the beginning of the method body but it didn't work.</div><div><br></div><div><br></div><div><br></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 dir="ltr"><div></div><div>HTH</div></div><div class="gmail_extra"><div><div class="h5"><br><div class="gmail_quote">On Tue, Jan 19, 2016 at 5:29 AM, <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-cb.1654.mcz" rel="noreferrer" target="_blank">http://source.squeak.org/VMMaker/VMMaker.oscog-cb.1654.mcz</a><br>
<br>
==================== Summary ====================<br>
<br>
Name: VMMaker.oscog-cb.1654<br>
Author: cb<br>
Time: 19 January 2016, 2:28:37.278 pm<br>
UUID: 35bdf68b-ea60-4f01-8799-14baa71ab416<br>
Ancestors: VMMaker.oscog-cb.1653<br>
<br>
In the SqueakV3Bytecode set, extendedStore and extStoreAndPop are now mapped as instVar and LiteralVar stores need to be mapped for immutability. We need to generate a fake mapping to make the mcpc to bcpc mapping happy.<br>
<br>
Starting from this version, the mcpc to bcpc is happy *except* for primitive error code. In this case, an extStore bytecode is used for the error code but is still not mapped.<br>
<br>
=============== Diff against VMMaker.oscog-cb.1653 ===============<br>
<br>
Item was changed:<br>
----- Method: SimpleStackBasedCogit>>extendedStoreAndPopBytecode (in category 'bytecode generators') -----<br>
extendedStoreAndPopBytecode<br>
| variableType variableIndex |<br>
variableType := byte1 >> 6 bitAnd: 3.<br>
variableIndex := byte1 bitAnd: 63.<br>
variableType = 0 ifTrue:<br>
[^self genStorePop: true ReceiverVariable: variableIndex].<br>
variableType = 1 ifTrue:<br>
+ [self genStorePop: true TemporaryVariable: variableIndex.<br>
+ "needs a fake map entry is Immutability is ON..."<br>
+ self cppIf: IMMUTABILITY ifTrue: [ self annotateBytecode: self Label. ].<br>
+ ^ 0].<br>
- [^self genStorePop: true TemporaryVariable: variableIndex].<br>
variableType = 3 ifTrue:<br>
[^self genStorePop: true LiteralVariable: variableIndex].<br>
^EncounteredUnknownBytecode!<br>
<br>
Item was changed:<br>
----- Method: SimpleStackBasedCogit>>extendedStoreBytecode (in category 'bytecode generators') -----<br>
extendedStoreBytecode<br>
| variableType variableIndex |<br>
variableType := byte1 >> 6 bitAnd: 3.<br>
variableIndex := byte1 bitAnd: 63.<br>
variableType = 0 ifTrue:<br>
[^self genStorePop: false ReceiverVariable: variableIndex].<br>
variableType = 1 ifTrue:<br>
+ [self genStorePop: false TemporaryVariable: variableIndex.<br>
+ "needs a fake map entry is Immutability is ON..."<br>
+ self cppIf: IMMUTABILITY ifTrue: [ self annotateBytecode: self Label. ].<br>
+ ^ 0].<br>
- [^self genStorePop: false TemporaryVariable: variableIndex].<br>
variableType = 3 ifTrue:<br>
[^self genStorePop: false LiteralVariable: variableIndex].<br>
^EncounteredUnknownBytecode!<br>
<br>
Item was changed:<br>
----- Method: StackToRegisterMappingCogit>>doubleExtendedDoAnythingBytecode (in category 'bytecode generators') -----<br>
doubleExtendedDoAnythingBytecode<br>
"Replaces the Blue Book double-extended send [132], in which the first byte was wasted on 8 bits of argument count.<br>
Here we use 3 bits for the operation sub-type (opType), and the remaining 5 bits for argument count where needed.<br>
The last byte give access to 256 instVars or literals.<br>
See also secondExtendedSendBytecode"<br>
| opType |<br>
opType := byte1 >> 5.<br>
opType = 0 ifTrue:<br>
[^self genSend: byte2 numArgs: (byte1 bitAnd: 31)].<br>
opType = 1 ifTrue:<br>
[^self genSendSuper: byte2 numArgs: (byte1 bitAnd: 31)].<br>
"We need a map entry for this bytecode for correct parsing.<br>
The sends will get an IsSend entry anyway. The other cases need a<br>
fake one. We could of course special case the scanning but that's silly."<br>
opType caseOf: {<br>
[2] -> [(coInterpreter isReadMediatedContextInstVarIndex: byte2)<br>
ifTrue: [self genPushMaybeContextReceiverVariable: byte2]<br>
ifFalse: [self genPushReceiverVariable: byte2.<br>
self ssTop annotateUse: true.<br>
^0]].<br>
[3] -> [self genPushLiteralIndex: byte2.<br>
self ssTop annotateUse: true.<br>
^0].<br>
[4] -> [self genPushLiteralVariable: byte2.].<br>
+ [7] -> [self genStorePop: false LiteralVariable: byte2.<br>
+ self cppIf: IMMUTABILITY ifTrue: [ "instruction is mapped" ^0 ] ] }<br>
- [7] -> [self genStorePop: false LiteralVariable: byte2] }<br>
otherwise: "5 & 6"<br>
[(coInterpreter isWriteMediatedContextInstVarIndex: byte2)<br>
ifTrue: [self genStorePop: opType = 6 MaybeContextReceiverVariable: byte2]<br>
+ ifFalse: [self genStorePop: opType = 6 ReceiverVariable: byte2].<br>
+ self cppIf: IMMUTABILITY ifTrue: [ "instruction is mapped" ^0 ]].<br>
- ifFalse: [self genStorePop: opType = 6 ReceiverVariable: byte2]].<br>
"We need a map entry for this bytecode for correct parsing (if the method builds a frame).<br>
We could of course special case the scanning but that's silly (or is it?)."<br>
self assert: needsFrame.<br>
"genPushMaybeContextInstVar, pushListVar, store & storePop all generate code"<br>
self assert: self prevInstIsPCAnnotated not.<br>
self annotateBytecode: self Label.<br>
^0!<br>
<br>
</blockquote></div><br><br clear="all"><div><br></div></div></div><span class=""><font color="#888888">-- <br><div><div dir="ltr"><div><span style="font-size:small;border-collapse:separate"><div>_,,,^..^,,,_<br></div><div>best, Eliot</div></span></div></div></div>
</font></span></div>
</blockquote></div><br></div></div>