<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">&lt;<a href="mailto:eliot.miranda@gmail.com" target="_blank">eliot.miranda@gmail.com</a>&gt;</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 &lt;-&gt; machine code mapping machinery isn&#39;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&#39;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 &gt;&gt; 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>&quot;needs a fake map entry is Immutability is ON...&quot;</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&#39;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 &gt;&gt; 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">        &quot;If </span>IMMUTABILITY<span style="white-space:pre-wrap"> is on then this bytecode could send back and so must be mapped.&quot;</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&#39;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&#39;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&#39;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&#39;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">&lt;<a href="mailto:commits@source.squeak.org" target="_blank">commits@source.squeak.org</a>&gt;</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&gt;&gt;extendedStoreAndPopBytecode (in category &#39;bytecode generators&#39;) -----<br>
  extendedStoreAndPopBytecode<br>
        | variableType variableIndex |<br>
        variableType := byte1 &gt;&gt; 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>
+               &quot;needs a fake map entry is Immutability is ON...&quot;<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&gt;&gt;extendedStoreBytecode (in category &#39;bytecode generators&#39;) -----<br>
  extendedStoreBytecode<br>
        | variableType variableIndex |<br>
        variableType := byte1 &gt;&gt; 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>
+               &quot;needs a fake map entry is Immutability is ON...&quot;<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&gt;&gt;doubleExtendedDoAnythingBytecode (in category &#39;bytecode generators&#39;) -----<br>
  doubleExtendedDoAnythingBytecode<br>
        &quot;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&quot;<br>
        | opType |<br>
        opType := byte1 &gt;&gt; 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>
        &quot;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&#39;s silly.&quot;<br>
        opType caseOf: {<br>
                        [2]     -&gt;      [(coInterpreter isReadMediatedContextInstVarIndex: byte2)<br>
                                                ifTrue: [self genPushMaybeContextReceiverVariable: byte2]<br>
                                                ifFalse: [self genPushReceiverVariable: byte2.<br>
                                                                self ssTop annotateUse: true.<br>
                                                                ^0]].<br>
                        [3]     -&gt;      [self genPushLiteralIndex: byte2.<br>
                                         self ssTop annotateUse: true.<br>
                                         ^0].<br>
                        [4]     -&gt;      [self genPushLiteralVariable: byte2.].<br>
+                       [7]     -&gt;      [self genStorePop: false LiteralVariable: byte2.<br>
+                                       self cppIf: IMMUTABILITY ifTrue: [ &quot;instruction is mapped&quot; ^0 ] ] }<br>
-                       [7]     -&gt;      [self genStorePop: false LiteralVariable: byte2] }<br>
                otherwise: &quot;5 &amp; 6&quot;<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: [ &quot;instruction is mapped&quot; ^0 ]].<br>
-                               ifFalse: [self genStorePop: opType = 6 ReceiverVariable: byte2]].<br>
        &quot;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&#39;s silly (or is it?).&quot;<br>
        self assert: needsFrame.<br>
        &quot;genPushMaybeContextInstVar, pushListVar, store &amp; storePop all generate code&quot;<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>