<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">2016-04-28 2:35 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:1px solid rgb(204,204,204);padding-left:1ex"> <br><div dir="ltr">Hi Nicolas,<div><br></div><div> I'm looking at this now. Your recent changes have also caused a huge number of warnings like this:</div><div><br></div><div><div>type mismatch for formal integerValue and actual "pwr - 1" when inlining pushInteger: in primitiveExponent. Use a cast.</div><div>type mismatch for formal reasonCode and actual "(objectMemory isIntegerObject: index) ifTrue: [PrimErrBadIndex] ifFalse: [PrimErrBadArgument]" when inlining primitiveFailFor: in primitiveFloatAt. Use a cast.</div><div>type mismatch for formal cond and actual "rcvr = arg" when inlining booleanCheat: in bytecodePrimIdentical. Use a cast.</div><div>type mismatch for formal cond and actual "rcvr = arg" when inlining booleanCheatSistaV1: in bytecodePrimIdenticalSistaV1. Use a cast.</div><div>type mismatch for formal cond and actual "rcvr = arg" when inlining booleanCheatV4: in bytecodePrimIdenticalV4. Use a cast.</div><div>type mismatch for formal reasonCode and actual "(objectMemory isIndexable: rcvr) ifTrue: [PrimErrBadIndex] ifFalse: [PrimErrBadReceiver]" when inlining primitiveFailFor: in commonVariable:at:put:cacheIndex:. Use a cast.</div><div>type mismatch for formal numSlots and actual "MessageLookupClassIndex + 1" when inlining eeInstantiateSmallClassIndex:format:numSlots: in createActualMessageTo:. Use a cast.</div><div>type mismatch for formal successBoolean and actual "integerArg ~= 0" when inlining success: in doPrimitiveDiv:by:. Use a cast.</div><div>type mismatch for formal successBoolean and actual "(result >> 60 + 1 bitAnd: 15) <= 1" when inlining success: in doPrimitiveDiv:by:. Use a cast.</div><div>type mismatch for formal successBoolean and actual "integerArg ~= 0" when inlining success: in doPrimitiveMod:by:. Use a cast.</div><div>type mismatch for formal successBoolean and actual "(integerResult >> 60 + 1 bitAnd: 15) <= 1" when inlining success: in doPrimitiveMod:by:. Use a cast.</div><div>type mismatch for formal successBoolean and actual "arg ~= 0.0" when inlining success: in primitiveFloatDivide:byArg:. Use a cast.</div><div>type mismatch for formal successBoolean and actual "objectsWritten = 1" when inlining success: in putLong:toFile:. Use a cast.</div><div>type mismatch for formal successBoolean and actual "objectsWritten = 1" when inlining success: in putShort:toFile:. Use a cast.</div><div>type mismatch for formal successBoolean and actual "objectsWritten = 1" when inlining success: in putWord32:toFile:. Use a cast.</div></div><div><br></div><div>:-(</div><div><br></div></div></blockquote><div>Great, I didn't know about this type checking. That's usefull.<br></div><div>But isn't the logic inverted?<br> self<br> variableOfType: (self typeFor: exprNode in: aTMethod)<br> acceptsValue: exprNode<br> ofType: (targetMethod typeFor: argName in: self)<br></div><div>I would expect<br> self<br> variableOfType: (targetMethod typeFor: argName in: self)<br> acceptsValue: exprNode<br> ofType: (self typeFor: exprNode in: aTMethod)</div><div><br></div><div>Unfortunately<br> someCharArray[i] = someExpressionPromotedToIntLikeBitAndFF & 0xFF;<br></div><div>would now complain because of int promotion...<br><br></div><div> <br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div></div><div>We have to be careful. Pharo is trying to release. I would like a stable VM. I would also like a better-inlined VM, so I support what you're doing. I'm just worried about timing.</div></div><div class="gmail_extra"><br></div></blockquote><div><br></div><div>Agree, I committed a bit too soon.<br></div><div>These changes are likely to cause a bit of unstability for a transition period.<br></div><div>Travis-CI bot seems back to green, but i have no idea of coverage.<br></div><div>In last resort we can revert some of the changes and apply only the important fixes.<br>Let's cross finger and hope it won't be necessary.<br><br></div><div> <br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div class="gmail_extra"><div class="gmail_quote">On Wed, Apr 27, 2016 at 12:47 PM, Nicolas Cellier <span dir="ltr"><<a href="mailto:nicolas.cellier.aka.nice@gmail.com" target="_blank">nicolas.cellier.aka.nice@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> <br><div dir="ltr"><div><div><div>Hi,<br></div><div>I changed the compiler flag -Wno-unused-value to -Wunused-value because there'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->thunkp)));<br> /* begin positive32BitIntegerFor: */<br> /* begin maybeInlinePositive32BitIntegerFor: */<br> assert(!((hasSixtyFourBitImmediates())));<br> if ((((unsigned int) value)) <= (MaxSmallInteger)) {<br> ((value << 1) | 1);<br></div><div> ^~~~~~~~~~~~~~ PROBLEM HERE: this result should be stored in variable 'object'<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> <inline: true><br> <var: #sp type: #'char *'><br> stackPages longAt: (sp := stackPointer - objectMemory wordSize) put: object.<br> stackPointer := sp<br><br></div><div>With positiveMachineIntegerFor:<br><br> <var: #value type: #'unsigned long'><br> <inline: true><br> ^objectMemory wordSize = 8<br> ifTrue: [self positive64BitIntegerFor: value]<br> ifFalse: [self positive32BitIntegerFor: value]<br></div><div><br></div><div>With positive32BitIntegerFor:<br><br> <inline: true><br> <var: 'integerValue' type: #'unsigned int'><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> <notOption: #Spur64BitMemoryManager><br> <var: 'integerValue' type: #'unsigned int'><br> | newLargeInteger |<br> self deny: objectMemory hasSixtyFourBitImmediates.<br> "force coercion because slang inliner sometimes incorrectly pass a signed int without converting to unsigned"<br> (self cCode: [self cCoerceSimple: integerValue to: #'unsigned int']<br> inSmalltalk: [integerValue bitAnd: 1 << 32 - 1]) <= objectMemory maxSmallInteger ifTrue:<br> [^objectMemory integerObjectOf: integerValue].<br> newLargeInteger := objectMemory<br> eeInstantiateSmallClassIndex: <br></div><div> ...snip...<br></div><div> ^newLargeInteger<br><br></div></div>
<br></blockquote></div><br><br clear="all"><div><br></div>-- <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>
</div>
<br></blockquote></div><br></div></div>