[squeak-dev] LiteralVariableNode>>sizeCodeForStore: #dubious and #bogus? (was: The Trunk: Tests-dtl.194.mcz)

Eliot Miranda eliot.miranda at gmail.com
Wed Apr 3 15:50:26 UTC 2013


Hi David,

On Tue, Apr 2, 2013 at 7:46 PM, David T. Lewis <lewis at mail.msen.com> wrote:

> On Fri, Mar 29, 2013 at 11:21:00AM -0400, David T. Lewis wrote:
> > On Fri, Mar 29, 2013 at 03:04:20PM +0000, Frank Shearar wrote:
> > > On 29 March 2013 14:17, David T. Lewis <lewis at mail.msen.com> wrote:
> > > >
> > > > I moved these two new tests from Inbox to Trunk. One test fails on
> recent
> > > > trunk images. I do not know if this is a bug or a feature, but I
> cannot
> > > > explain the regression so I don't want to assume it's a feature.
> > > >
> > > > Dave
> > >
> > >
> http://build.squeak.org/job/SqueakTrunk/237/testReport/junit/Tests.Compiler/CompilerTest/testMaxLiteralsWithClassReferenceInClosure/
> > >
> > > shows the test failure.
> >
> > The actual change to the image that causes this test failure occurred
> between
> > SqueakTrunk/212 and SqueakTrunk/213. In other words, if this test had
> been
> > part of the test suite all along, then you would have seen the failure
> appear
> > in 213.
> >
> > I suspect that this is a side effect of class lookups associated with
> the Environments
> > changes, but I cannot say if it is a bug or an explainable variation.
> Either way
> > it's good to note the change, and we can either fix the bug or fix the
> test
> > as needed.
>
> (self flag: #dubious and: [self flag: #bogus]) ifTrue: [self
> askForGuidance]
>
> LiteralVariableNode>>sizeCodeForStore: is flagged thus, which I suspect
> is somehow related to the apparently incorrect warning about too many
> literals.
>

I doubt it.  How to reproduce the incorrect warning?  The point
about LiteralVariableNode>>sizeCodeForStore:
(& LiteralVariableNode>>emitCodeForStore:encoder:) is that they don't do
what's advertised.  i.e. they don't leave the value of the expression that
is stored on top of stack unless the special write binding answers the
expression in response to #value:.  So if one were to write

     v := SomeSpecialBinding := expr

v would end up holding SomeSpecialBinding, not expr.

This could perhaps be corrected by ensuring that all classes that implement
isSpecialWriteBinding also implement #value: to answer the argument, not
the receiver.  e.g. instead of

Binding methods for accessing
value: anObject
(AttemptToWriteReadOnlyGlobal signal: 'Cannot store into read-only
bindings')
ifTrue: [value := anObject]

we use

value: anObject
(AttemptToWriteReadOnlyGlobal signal: 'Cannot store into read-only
bindings')
ifTrue: [value := anObject].
^anObject

but that's tricky.

The problem for the code generator is that there's no swap operation.  e.g.
the following would work:

stack before:
    expr

first half of code:
    dup
    push binding
    swap

stack during:
    expr
    binding
    expr

second half of code
    send #value:
    pop

stack after
    expr

but that's a) a new bytecode and b) a lot of bytes to do a store.

A different approach would be to rewrite emit because it knows the stack
pointer to write something like
    push binding
    push temp N (where N is the offset of expr on the stack)
    send value:

I think this is the correct approach.  It might require fixing the
decompiler, but it shouldn't be hard.  I'll have a go.  In any case we need
a test that ensures that

    | v expr |
    expr := Object new.
    v := SomeSpecialBinding := expr.
    self assert: v == expr




> Reducing to a smaller test case,  compiling 'Time now' yields a parse tree
> with LiteralVariableNode {Time} that has code -4 which causes dual
> reserving
> in sizeCodeForValue: and this in turn leads to a mis-counting of literals
> in the encoder.
>
> In an earlier version of trunk, the same LiteralVariableNode has a
> non-negative
> code value that does not produce the dual reserving in sizeCodeForValue:
>
> Dave
>
>
>


-- 
best,
Eliot
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.squeakfoundation.org/pipermail/squeak-dev/attachments/20130403/e49b0597/attachment.htm


More information about the Squeak-dev mailing list