<div dir="ltr"><div><div>It sounds like related to ifTrue:ifFalse: alternatives.<br></div>When entering pass 6 inlined is still correct:<br><br>    &quot;begin push:&quot;.<br>    objectMemory wordSize = 8 ifTrue: [object3 := self positive64BitIntegerFor: vmCallbackContext asUnsignedInteger] ifFalse: [<br>        &quot;begin positive32BitIntegerFor:&quot;.<br>        false ifTrue: [<br>            object3 := objectMemory integerObjectOf: (vmCallbackContext asUnsignedInteger asUnsignedLong bitAnd: 4294967295).<br>            goto l1<br>        ] ifFalse: [<br>            object3 := self maybeInlinePositive32BitIntegerFor: vmCallbackContext asUnsignedInteger.<br>            goto l1<br>        ].<br>        l1:    &quot;end positive32BitIntegerFor:&quot;<br>    ].<br>    stackPages longAt: sp4 := stackPointer - objectMemory wordSize put: object3.<br>    stackPointer := sp4.<br><br></div><div>The bad step happens before entering in pass 7. It correctly applies to above instance of this message send with object3 := assignment distributed in each ifTrue:ifFalse: returning branch:<br><br>objectMemory wordSize = 8 ifTrue: [object3 := self positive64BitIntegerFor: vmCallbackContext asUnsignedInteger] ifFalse: [<br>        &quot;begin positive32BitIntegerFor:&quot;.<br>        false ifTrue: [<br>            object3 := objectMemory integerObjectOf: (vmCallbackContext asUnsignedInteger asUnsignedLong bitAnd: 4294967295).<br>            goto l1<br>        ] ifFalse: [<br>            &quot;begin maybeInlinePositive32BitIntegerFor:&quot;.<br>            self deny: objectMemory hasSixtyFourBitImmediates.<br>            (self cCoerceSimple: vmCallbackContext asUnsignedInteger to: unsigned int) &lt;= objectMemory maxSmallInteger ifTrue: [<br>                object3 := objectMemory integerObjectOf: vmCallbackContext asUnsignedInteger.<br>                goto l20<br>            ].<br>            &quot;begin eeInstantiateSmallClassIndex:format:numSlots:&quot;.<br>            objFormat3 := self firstByteFormat + (8 - 4 bitAnd: self wordSize - 1).<br>            self assert: (1 &gt;= 0 and: [ClassLargePositiveIntegerCompactIndex ~= 0]).<br>            self assert: (objFormat3 &lt; self firstByteFormat ifTrue: [objFormat3] ifFalse: [objFormat3 bitAnd: self byteFormatMask]) = (self instSpecOfClass: (self knownClassAtIndex: ClassLargePositiveIntegerCompactIndex)).<br>            &quot;begin allocateSmallNewSpaceSlots:format:classIndex:&quot;.<br>            self assert: 1 &lt; self numSlotsMask.<br>            newObj3 := freeStart.<br>            numBytes3 := self baseHeaderSize + (1 &lt;= 1 ifTrue: [8] ifFalse: [1 + (1 bitAnd: 1) * self bytesPerOop]).<br>            self assert: numBytes3 \\ self allocationUnit = 0.<br>            self assert: newObj3 \\ self allocationUnit = 0.<br>            freeStart + numBytes3 &gt; scavengeThreshold ifTrue: [<br>                needGCFlag ifFalse: [<br>                    &quot;begin scheduleScavenge&quot;.<br>                    needGCFlag := true.<br>                    coInterpreter forceInterruptCheck<br>                ].<br>                freeStart + numBytes3 &gt; scavenger eden limit ifTrue: [<br>                    self error: &#39;no room in eden for allocateSmallNewSpaceSlots:format:classIndex:&#39;.<br>                    newLargeInteger3 := 0.<br>                    goto l19<br>                ]<br>            ].<br>            self long64At: newObj3 put: (self cCoerceSimple: 1 to: usqLong) &lt;&lt; self numSlotsFullShift + objFormat3 &lt;&lt; self formatShift + ClassLargePositiveIntegerCompactIndex.<br>            freeStart := freeStart + numBytes3.<br>            newLargeInteger3 := newObj3.<br>            l19:    &quot;end allocateSmallNewSpaceSlots:format:classIndex:&quot;.<br>            self cppIf: SPURVM ifTrue: [<br>                self long32At: newLargeInteger3 + self baseHeaderSize + 0 &lt;&lt; 2 put: (objectMemory byteSwapped32IfBigEndian: vmCallbackContext asUnsignedInteger).<br>                self long32At: newLargeInteger3 + self baseHeaderSize + 1 &lt;&lt; 2 put: 0<br>            ] ifFalse: [self long32At: newLargeInteger3 + self baseHeaderSize + 0 &lt;&lt; 2 put: (objectMemory byteSwapped32IfBigEndian: vmCallbackContext asUnsignedInteger)].<br>            object3 := newLargeInteger3.<br>            l20:    &quot;end maybeInlinePositive32BitIntegerFor:&quot;.<br>            goto l1<br>        ].<br>        l1:    &quot;end positive32BitIntegerFor:&quot;<br>    ]<br><br></div><div>However, at same stage, the transformation also applies to previous instance of same message, but this time without correct distribution of object := assignment.<br></div><div><br>        &quot;begin push:&quot;.<br>        &quot;begin positiveMachineIntegerFor:&quot;.<br>        value := vmCallbackContext thunkp asUnsignedInteger.<br>        object := objectMemory wordSize = 8 ifTrue: [self positive64BitIntegerFor: value] ifFalse: [<br>                    &quot;begin positive32BitIntegerFor:&quot;.<br>                    false ifTrue: [<br>                        objectMemory integerObjectOf: (value asUnsignedLong bitAnd: 4294967295).<br>                        goto l5<br>                    ] ifFalse: [<br>                        &quot;begin maybeInlinePositive32BitIntegerFor:&quot;.<br>                        self deny: objectMemory hasSixtyFourBitImmediates.<br>                        (self cCoerceSimple: value to: unsigned int) &lt;= objectMemory maxSmallInteger ifTrue: [<br>                            objectMemory integerObjectOf: value.<br>                            goto l4<br>                        ].<br>                        &quot;begin eeInstantiateSmallClassIndex:format:numSlots:&quot;.<br>                        objFormat := self firstByteFormat + (8 - 4 bitAnd: self wordSize - 1).<br>                        self assert: (1 &gt;= 0 and: [ClassLargePositiveIntegerCompactIndex ~= 0]).<br>                        self assert: (objFormat &lt; self firstByteFormat ifTrue: [objFormat] ifFalse: [objFormat bitAnd: self byteFormatMask]) = (self instSpecOfClass: (self knownClassAtIndex: ClassLargePositiveIntegerCompactIndex)).<br>                        &quot;begin allocateSmallNewSpaceSlots:format:classIndex:&quot;.<br>                        self assert: 1 &lt; self numSlotsMask.<br>                        newObj := freeStart.<br>                        numBytes := self baseHeaderSize + (1 &lt;= 1 ifTrue: [8] ifFalse: [1 + (1 bitAnd: 1) * self bytesPerOop]).<br>                        self assert: numBytes \\ self allocationUnit = 0.<br>                        self assert: newObj \\ self allocationUnit = 0.<br>                        freeStart + numBytes &gt; scavengeThreshold ifTrue: [<br>                            needGCFlag ifFalse: [<br>                                &quot;begin scheduleScavenge&quot;.<br>                                needGCFlag := true.<br>                                coInterpreter forceInterruptCheck<br>                            ].<br>                            freeStart + numBytes &gt; scavenger eden limit ifTrue: [<br>                                self error: &#39;no room in eden for allocateSmallNewSpaceSlots:format:classIndex:&#39;.<br>                                newLargeInteger := 0.<br>                                goto l6<br>                            ]<br>                        ].<br>                        self long64At: newObj put: (self cCoerceSimple: 1 to: usqLong) &lt;&lt; self numSlotsFullShift + objFormat &lt;&lt; self formatShift + ClassLargePositiveIntegerCompactIndex.<br>                        freeStart := freeStart + numBytes.<br>                        newLargeInteger := newObj.<br>                        l6:    &quot;end allocateSmallNewSpaceSlots:format:classIndex:&quot;.<br>                        self cppIf: SPURVM ifTrue: [<br>                            self long32At: newLargeInteger + self baseHeaderSize + 0 &lt;&lt; 2 put: (objectMemory byteSwapped32IfBigEndian: value).<br>                            self long32At: newLargeInteger + self baseHeaderSize + 1 &lt;&lt; 2 put: 0<br>                        ] ifFalse: [self long32At: newLargeInteger + self baseHeaderSize + 0 &lt;&lt; 2 put: (objectMemory byteSwapped32IfBigEndian: value)].<br>                        newLargeInteger.<br>                        l4:    &quot;end maybeInlinePositive32BitIntegerFor:&quot;.<br>                        goto l5<br>                    ].<br>                    l5:    &quot;end positive32BitIntegerFor:&quot;<br>                ].<br>        stackPages longAt: sp := stackPointer - objectMemory wordSize put: object.<br>        stackPointer := sp.<br><br></div><div>What sounds strange is that previous instance of that message was really behind when entering pass 6:<br><br>        &quot;begin push:&quot;.<br>        object := self positiveMachineIntegerFor: vmCallbackContext thunkp asUnsignedInteger.<br>        stackPages longAt: sp := stackPointer - objectMemory wordSize put: object.<br>        stackPointer := sp.<br><br></div><div> It sounds like very tricky circumstances.<br>My finding is that this is due to a global state mutation (Tmethod isComplete)<br>This is used in <br><br>inlineableSend: aNode in: aCodeGen<br>    &quot;Answer true if the given send node is a call to a method that can be inlined.&quot;<br><br>    | m |<br>    aCodeGen maybeBreakForTestToInline: aNode in: self.<br>    aNode isSend ifFalse: [ ^false ].<br>    m := aCodeGen methodNamed: aNode selector.  &quot;nil if builtin or external function&quot;<br>    ^m ~= nil and: [m ~~ self and: [m isComplete and: [aCodeGen mayInline: m selector]]]<br><br></div><div>Now, thanks to this global state, we inline more than the very restrictive rules, but unfortunately sometimes wrongly...<br><br></div></div><div class="gmail_extra"><br><div class="gmail_quote">2016-04-27 21:47 GMT+02:00 Nicolas Cellier <span dir="ltr">&lt;<a href="mailto:nicolas.cellier.aka.nice@gmail.com" target="_blank">nicolas.cellier.aka.nice@gmail.com</a>&gt;</span>:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div><div><div>Hi,<br></div><div>I changed the compiler flag -Wno-unused-value to -Wunused-value because there&#39;s no reason to carry dead code. Dead code should ring a bell.<br></div>it seems it uncovers a problem in code generation.<br></div>An example is in code generated for sendInvokeCallbackContext:<br><br>       if ((argumentCountOfMethodHeader(methodHeaderOf(GIV(newMethod)))) == 4) {<br>                /* begin push: */<br>                value = ((usqInt)((vmCallbackContext-&gt;thunkp)));<br>                /* begin positive32BitIntegerFor: */<br>                /* begin maybeInlinePositive32BitIntegerFor: */<br>                assert(!((hasSixtyFourBitImmediates())));<br>                if ((((unsigned int) value)) &lt;= (MaxSmallInteger)) {<br>                        ((value &lt;&lt; 1) | 1);<br></div><div>                       ^~~~~~~~~~~~~~ PROBLEM HERE: this result should be stored in variable &#39;object&#39;<br></div><div>                        goto l4;<br>                }<br>                /* begin eeInstantiateSmallClassIndex:format:numSlots: */<br></div>...snip...<br><div>                object = newLargeInteger;<br>        l4:     /* end maybeInlinePositive32BitIntegerFor: */;<br>                goto l5;<br><br>        l5:     /* end positive32BitIntegerFor: */;<br><br>                longAtput((sp = GIV(stackPointer) - BytesPerWord), object);<br> <br></div><div>The corresponding Slang code is:<br><br>    (self argumentCountOf: newMethod) = 4 ifTrue:<br>        [self push: (self positiveMachineIntegerFor: vmCallbackContext thunkp asUnsignedInteger).<br><br></div><div>With push:<br><br>    | sp |<br>    &lt;inline: true&gt;<br>    &lt;var: #sp type: #&#39;char *&#39;&gt;<br>    stackPages longAt: (sp := stackPointer - objectMemory wordSize) put: object.<br>    stackPointer := sp<br><br></div><div>With positiveMachineIntegerFor:<br><br>    &lt;var: #value type: #&#39;unsigned long&#39;&gt;<br>    &lt;inline: true&gt;<br>    ^objectMemory wordSize = 8<br>        ifTrue: [self positive64BitIntegerFor: value]<br>        ifFalse: [self positive32BitIntegerFor: value]<br></div><div><br></div><div>With positive32BitIntegerFor:<br><br>    &lt;inline: true&gt;<br>    &lt;var: &#39;integerValue&#39; type: #&#39;unsigned int&#39;&gt;<br>    objectMemory hasSixtyFourBitImmediates<br>        ifTrue:<br>            [^objectMemory integerObjectOf: (integerValue asUnsignedLong bitAnd: 16rFFFFFFFF)]<br>        ifFalse:<br>            [^self maybeInlinePositive32BitIntegerFor: integerValue]<br><br></div><div>With maybeInlinePositive32BitIntegerFor:<br><br>    &lt;notOption: #Spur64BitMemoryManager&gt;<br>    &lt;var: &#39;integerValue&#39; type: #&#39;unsigned int&#39;&gt;<br>    | newLargeInteger |<br>    self deny: objectMemory hasSixtyFourBitImmediates.<br>       &quot;force coercion because slang inliner sometimes incorrectly pass a signed int without converting to unsigned&quot;<br>       (self cCode: [self cCoerceSimple: integerValue to: #&#39;unsigned int&#39;]<br>            inSmalltalk: [integerValue bitAnd: 1 &lt;&lt; 32 - 1]) &lt;= objectMemory maxSmallInteger ifTrue:<br>        [^objectMemory integerObjectOf: integerValue].<br>    newLargeInteger := objectMemory<br>                            eeInstantiateSmallClassIndex: <br></div><div>    ...snip...<br></div><div>    ^newLargeInteger<br><br></div></div>
</blockquote></div><br></div>