[Vm-dev] Serious inlining problem
Nicolas Cellier
nicolas.cellier.aka.nice at gmail.com
Thu Apr 28 07:06:35 UTC 2016
2016-04-28 2:35 GMT+02:00 Eliot Miranda <eliot.miranda at gmail.com>:
>
> Hi Nicolas,
>
> I'm looking at this now. Your recent changes have also caused a huge
> number of warnings like this:
>
> type mismatch for formal integerValue and actual "pwr - 1" when inlining
> pushInteger: in primitiveExponent. Use a cast.
> type mismatch for formal reasonCode and actual "(objectMemory
> isIntegerObject: index) ifTrue: [PrimErrBadIndex] ifFalse:
> [PrimErrBadArgument]" when inlining primitiveFailFor: in primitiveFloatAt.
> Use a cast.
> type mismatch for formal cond and actual "rcvr = arg" when inlining
> booleanCheat: in bytecodePrimIdentical. Use a cast.
> type mismatch for formal cond and actual "rcvr = arg" when inlining
> booleanCheatSistaV1: in bytecodePrimIdenticalSistaV1. Use a cast.
> type mismatch for formal cond and actual "rcvr = arg" when inlining
> booleanCheatV4: in bytecodePrimIdenticalV4. Use a cast.
> 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.
> type mismatch for formal numSlots and actual "MessageLookupClassIndex + 1"
> when inlining eeInstantiateSmallClassIndex:format:numSlots: in
> createActualMessageTo:. Use a cast.
> type mismatch for formal successBoolean and actual "integerArg ~= 0" when
> inlining success: in doPrimitiveDiv:by:. Use a cast.
> type mismatch for formal successBoolean and actual "(result >> 60 + 1
> bitAnd: 15) <= 1" when inlining success: in doPrimitiveDiv:by:. Use a cast.
> type mismatch for formal successBoolean and actual "integerArg ~= 0" when
> inlining success: in doPrimitiveMod:by:. Use a cast.
> type mismatch for formal successBoolean and actual "(integerResult >> 60 +
> 1 bitAnd: 15) <= 1" when inlining success: in doPrimitiveMod:by:. Use a
> cast.
> type mismatch for formal successBoolean and actual "arg ~= 0.0" when
> inlining success: in primitiveFloatDivide:byArg:. Use a cast.
> type mismatch for formal successBoolean and actual "objectsWritten = 1"
> when inlining success: in putLong:toFile:. Use a cast.
> type mismatch for formal successBoolean and actual "objectsWritten = 1"
> when inlining success: in putShort:toFile:. Use a cast.
> type mismatch for formal successBoolean and actual "objectsWritten = 1"
> when inlining success: in putWord32:toFile:. Use a cast.
>
> :-(
>
> Great, I didn't know about this type checking. That's usefull.
But isn't the logic inverted?
self
variableOfType: (self typeFor: exprNode in: aTMethod)
acceptsValue: exprNode
ofType: (targetMethod typeFor: argName in: self)
I would expect
self
variableOfType: (targetMethod typeFor: argName in: self)
acceptsValue: exprNode
ofType: (self typeFor: exprNode in: aTMethod)
Unfortunately
someCharArray[i] = someExpressionPromotedToIntLikeBitAndFF & 0xFF;
would now complain because of int promotion...
> 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.
>
>
Agree, I committed a bit too soon.
These changes are likely to cause a bit of unstability for a transition
period.
Travis-CI bot seems back to green, but i have no idea of coverage.
In last resort we can revert some of the changes and apply only the
important fixes.
Let's cross finger and hope it won't be necessary.
> On Wed, Apr 27, 2016 at 12:47 PM, Nicolas Cellier <
> nicolas.cellier.aka.nice at gmail.com> wrote:
>
>>
>> Hi,
>> 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.
>> it seems it uncovers a problem in code generation.
>> An example is in code generated for sendInvokeCallbackContext:
>>
>> if ((argumentCountOfMethodHeader(methodHeaderOf(GIV(newMethod))))
>> == 4) {
>> /* begin push: */
>> value = ((usqInt)((vmCallbackContext->thunkp)));
>> /* begin positive32BitIntegerFor: */
>> /* begin maybeInlinePositive32BitIntegerFor: */
>> assert(!((hasSixtyFourBitImmediates())));
>> if ((((unsigned int) value)) <= (MaxSmallInteger)) {
>> ((value << 1) | 1);
>> ^~~~~~~~~~~~~~ PROBLEM HERE: this result should be
>> stored in variable 'object'
>> goto l4;
>> }
>> /* begin eeInstantiateSmallClassIndex:format:numSlots: */
>> ...snip...
>> object = newLargeInteger;
>> l4: /* end maybeInlinePositive32BitIntegerFor: */;
>> goto l5;
>>
>> l5: /* end positive32BitIntegerFor: */;
>>
>> longAtput((sp = GIV(stackPointer) - BytesPerWord),
>> object);
>>
>> The corresponding Slang code is:
>>
>> (self argumentCountOf: newMethod) = 4 ifTrue:
>> [self push: (self positiveMachineIntegerFor: vmCallbackContext
>> thunkp asUnsignedInteger).
>>
>> With push:
>>
>> | sp |
>> <inline: true>
>> <var: #sp type: #'char *'>
>> stackPages longAt: (sp := stackPointer - objectMemory wordSize) put:
>> object.
>> stackPointer := sp
>>
>> With positiveMachineIntegerFor:
>>
>> <var: #value type: #'unsigned long'>
>> <inline: true>
>> ^objectMemory wordSize = 8
>> ifTrue: [self positive64BitIntegerFor: value]
>> ifFalse: [self positive32BitIntegerFor: value]
>>
>> With positive32BitIntegerFor:
>>
>> <inline: true>
>> <var: 'integerValue' type: #'unsigned int'>
>> objectMemory hasSixtyFourBitImmediates
>> ifTrue:
>> [^objectMemory integerObjectOf: (integerValue asUnsignedLong
>> bitAnd: 16rFFFFFFFF)]
>> ifFalse:
>> [^self maybeInlinePositive32BitIntegerFor: integerValue]
>>
>> With maybeInlinePositive32BitIntegerFor:
>>
>> <notOption: #Spur64BitMemoryManager>
>> <var: 'integerValue' type: #'unsigned int'>
>> | newLargeInteger |
>> self deny: objectMemory hasSixtyFourBitImmediates.
>> "force coercion because slang inliner sometimes incorrectly pass a
>> signed int without converting to unsigned"
>> (self cCode: [self cCoerceSimple: integerValue to: #'unsigned int']
>> inSmalltalk: [integerValue bitAnd: 1 << 32 - 1]) <=
>> objectMemory maxSmallInteger ifTrue:
>> [^objectMemory integerObjectOf: integerValue].
>> newLargeInteger := objectMemory
>> eeInstantiateSmallClassIndex:
>> ...snip...
>> ^newLargeInteger
>>
>>
>>
>
>
> --
> _,,,^..^,,,_
> best, Eliot
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.squeakfoundation.org/pipermail/vm-dev/attachments/20160428/045ea821/attachment-0001.htm
More information about the Vm-dev
mailing list