Incomplete deoptimization of #ifNotNil: and #ifNotNil:ifNil:

> Hi All,
>
> There's a bug in the compiler which should be easy to fix for  
> someone familiar with the code.
> The following expression returns #(nil nil true true) instead of  
> #(nil nil nil nil):
>
> {
> nil
> ifNil: [ 1 ] ifNotNil: [ 2 ];
> yourself.
> nil
> ifNil: [ 1 ];
> yourself.
> nil
> ifNotNil: [ 2 ];
> yourself.
> nil
> ifNotNil: [ 2 ] ifNil: [ 1 ];
> yourself.
> }
>
> The cause of the problem is that in case of #ifNotNil: and  
> #ifNotNil:ifNil: the deoptimization doesn't clear all previously  
> generated optimized bytecodes.
> E.g. in case of #ifNotNil: instead of
>
> <73> pushConstant: nil
> <88> dup
> <8F 00 00 02> closureNumCopied: 0 numArgs: 0 bytes 33 to 34
>     <76> pushConstant: 1
>     <7D> blockReturn
> <E0> send: ifNotNil:
> <87> pop
> <D1> send: yourself
>
> we get
>
> <73> pushConstant: nil
> <73> pushConstant: nil
> <C6> send: ==
> <88> dup
> <8F 00 00 02> closureNumCopied: 0 numArgs: 0 bytes 33 to 34
>     <76> pushConstant: 1
>     <7D> blockReturn
> <E0> send: ifNotNil:
> <87> pop
> <D1> send: yourself
>
> It would be great to write some tests which cover all such cases  
> (including other methods), but I'm not sure what form is the best  
> for this. Simple tests which just verify some values? Or something  
> that checks the generated bytecodes?
>
> Levente

> Found the problem. Unlike the other optimizations, optimizing ifNil  
> replaces the original receiver of the MessageNode by {rcvr == nil}.  
> But it is not restored when deoptimizing.
>
> We can fix it easily by restoring the original receiver in  
> #ensureCanCascade:. I just did that in Compiler-bf.293.
>
> - Bert -
>
> On 19.02.2015, at 17:50, Bert Freudenberg <[hidden email]> wrote:
>>
>> It looks like Eliot removed the check in 2011. Before that it read:
>>
>> canCascade
>> ^(receiver == NodeSuper or: [special > 0]) not
>>
>> - Bert -
>>
>>> On 19.02.2015, at 17:35, J. Vuletich (mail lists)  
>>> <[hidden email]> wrote:
>>>
>>> Hi Levente,
>>>
>>> Thanks for raising this. I don't know how to fix it, but in Cuis,  
>>> I've just disabled these selectors in #canCascade (#ifNil: too,  
>>> even if not really needed). I recompiled the system afterwards  
>>> without error notifications. And I see no real value in this  
>>> coding style, so I see no downside in just disallowing it.
>>>
>>> Cheers,
>>> Juan Vuletich
>>>
>>> Quoting Levente Uzonyi <[hidden email]>:
>>>
>>>> Hi All,
>>>>
>>>> There's a bug in the compiler which should be easy to fix for  
>>>> someone familiar with the code.
>>>> The following expression returns #(nil nil true true) instead of  
>>>> #(nil nil nil nil):
>>>>
>>>> {
>>>> nil
>>>> ifNil: [ 1 ] ifNotNil: [ 2 ];
>>>> yourself.
>>>> nil
>>>> ifNil: [ 1 ];
>>>> yourself.
>>>> nil
>>>> ifNotNil: [ 2 ];
>>>> yourself.
>>>> nil
>>>> ifNotNil: [ 2 ] ifNil: [ 1 ];
>>>> yourself.
>>>> }
>>>>
>>>> The cause of the problem is that in case of #ifNotNil: and  
>>>> #ifNotNil:ifNil: the deoptimization doesn't clear all previously  
>>>> generated optimized bytecodes.
>>>> E.g. in case of #ifNotNil: instead of
>>>>
>>>> <73> pushConstant: nil
>>>> <88> dup
>>>> <8F 00 00 02> closureNumCopied: 0 numArgs: 0 bytes 33 to 34
>>>>   <76> pushConstant: 1
>>>>   <7D> blockReturn
>>>> <E0> send: ifNotNil:
>>>> <87> pop
>>>> <D1> send: yourself
>>>>
>>>> we get
>>>>
>>>> <73> pushConstant: nil
>>>> <73> pushConstant: nil
>>>> <C6> send: ==
>>>> <88> dup
>>>> <8F 00 00 02> closureNumCopied: 0 numArgs: 0 bytes 33 to 34
>>>>   <76> pushConstant: 1
>>>>   <7D> blockReturn
>>>> <E0> send: ifNotNil:
>>>> <87> pop
>>>> <D1> send: yourself
>>>>
>>>> It would be great to write some tests which cover all such cases  
>>>> (including other methods), but I'm not sure what form is the best  
>>>> for this. Simple tests which just verify some values? Or  
>>>> something that checks the generated bytecodes?
>>>>
>>>> Levente
>>
>>
>>
>>