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

[Clément Béra]

Hi,

answers inlined

On Wed, Oct 10, 2018 at 1:40 AM 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 :-) )
>
>
It looks good to me. This can be integrated.
This is versioned in Monticello. So I guess you can commit if you have the
right and if there's a problem we will ignore the commit and tell on the
mailing list why.


>  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
>


-- 
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/fa3b8e62/attachment.html>