[Vm-dev] Suggested change for SpurMemoryManager>>#classAtIndex:

[Clément Béra]

Hi again,

answers inlined again.

On Wed, Oct 10, 2018 at 11:45 AM Hernan Wilkinson <
hernan.wilkinson at 10pines.com> wrote:

>
> well, I have the same problem but with the message #lengthOf:, but now the
> behavior is weird.
> If I have the following code, everything works fine:
> typesSize := (objectMemory lengthOf: types) - 1.
>
> If I remove the - 1, like this:
> typesSize := objectMemory lengthOf: types.
>
> I get redefinition of labels errors:
> ../../spurstack64src/vm/gcc3x-interp.c:4879:2: error: redefinition of
> label 'l46'
>         l46:    /* end lengthOf:format: */;
>         ^
> ../../spurstack64src/vm/gcc3x-interp.c:4693:2: note: previous definition
> is here
>         l46:    /* end lengthOf:format: */;
>         ^
>
> The thing is that in the first case #lengthOf: is not inlined but in the
> second it is and it looks like that the label is not regenerated every time
> it gets inlined.
>  Is there a well know workaround for this? I mean, I could do:
> typesSize := (objectMemory lengthOf: types) - 0.
>

Looks like a bug in the slang inliner. The Slang to C compiler is able to
compile the VM, and that's it, so if you do anything that's not already
used, it basically has undefined behavior (may or may not work). That's
probably known and it would be better if Eliot answers about that.

>
>  and it will compile but that is not the point.
>  Also, why in the first case the code generator decides not to inline
> #lengthOf: but it does in the second case?
>
> There's two inlining strategies. On the JIT side it's explicit, so inline:
true will inline, anything else is not inlined. The key aspect there is to
inline non C compatible code (closures, alloca interactions). On the
interpreter side it's heuristic based. It's quite tricky because you want
to control carefully what is inlined in the main interpreter loop & on
performance critical functions, while keeping enough information for
profiling. You can fine-tune the heuristics with inline: false/true. You
can look into the details in the CCodeGenerator class. Then there are the
work-arounds, for instance since linking time optimizations are not
enabled, quick calls across files are typically defined as macros to be
inlined. Again, it would be better if Eliot answers about that, he has
better knowledge on this.


Thanks!
> Hernan.
>
>
>
> On Tue, Oct 9, 2018 at 8:40 PM Hernan Wilkinson <
> hernan.wilkinson at 10pines.com> wrote:
>
>> Hi,
>>  I got compile errors when setting <inline: true> in a method I created.
>>  The errors where a label redefinition such as:
>> ../../spurstack64src/vm/gcc3x-interp.c:4790:2: error: redefinition of
>> label 'l35'
>>         l35:    /* end classAtIndex: */;
>>         ^
>> ../../spurstack64src/vm/gcc3x-interp.c:4634:2: note: previous definition
>> is here
>>         l35:    /* end classAtIndex: */;
>>         ^
>>
>>  After some research I concluded that the problem was
>> at SpurMemoryManager>>#classAtIndex: due to how it returns:
>> ...
>> classTablePage = nilObj ifTrue:
>> [^nil].
>> ^self
>> fetchPointer: (classIndex bitAnd: self classTableMinorIndexMask)
>> ofObject: classTablePage
>>
>>  I think the source code generator does not support correctly returning
>> at different points when inlining the same method more than once.
>>  So I changed that implementation to:
>> ...
>> ^classTablePage = nilObj
>> ifTrue: [nil]
>> ifFalse: [ self
>> fetchPointer: (classIndex bitAnd: self classTableMinorIndexMask)
>> ofObject: classTablePage ]
>>
>>  Doing so the compile errors were gone and the VM keeps working correctly.
>>  Is this fix correct or does it have some performance hit I do not know?
>>  Should I inform about this change with a PR? (Sorry about this question
>> but I'm new to this vm stuff :-) )
>>
>>  Here is the complete code of the method:
>> classAtIndex: classIndex
>> <api>
>> <inline: true>
>> | classTablePage |
>> self assert: (classIndex >= 0 and: [classIndex <= self tagMask or:
>> [classIndex >= self arrayClassIndexPun and: [classIndex <= self
>> classIndexMask]]]).
>> classTablePage := self fetchPointer: classIndex >> self
>> classTableMajorIndexShift
>> ofObject: hiddenRootsObj.
>> ^classTablePage = nilObj
>> ifTrue: [nil]
>> ifFalse: [ self
>> fetchPointer: (classIndex bitAnd: self classTableMinorIndexMask)
>> ofObject: classTablePage ]
>>
>>  Cheers!
>>  Hernan.
>> --
>>
>> *Hernán WilkinsonAgile Software Development, Teaching & Coaching*
>> *Phone: +54-011*-4893-2057
>> *Twitter: @HernanWilkinson*
>> *site: http://www.10Pines.com <http://www.10pines.com/>*
>> Address: Alem 896, Floor 6, Buenos Aires, Argentina
>>
>
>
> --
>
> *Hernán WilkinsonAgile Software Development, Teaching & Coaching*
> *Phone: +54-011*-4893-2057
> *Twitter: @HernanWilkinson*
> *site: http://www.10Pines.com <http://www.10pines.com/>*
> Address: Alem 896, Floor 6, Buenos Aires, Argentina
>


-- 
Clément Béra
https://clementbera.github.io/
https://clementbera.wordpress.com/
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.squeakfoundation.org/pipermail/vm-dev/attachments/20181011/d4e8520c/attachment-0001.html>