Request for reviews (S): 6820514: meet not symmetric failure in ctw

[Tom Rodriguez]

On Mar 23, 2009, at 2:14 PM, Vladimir Kozlov wrote:

> No, it is not true. The original comment's line is still true

Sorry, my original comment about what it changed it to was wrong.  I'd  
rewritten this several times and forgot to correct that.  The original  
comment was wrong and it's still wrong for your change.

>
>
> >       //  AnyNull |  U-AN     
> |................Unloaded......................|
>
> meet_ptr() maps almost everything to the original PRT:
>
> >   //              TopPTR,    AnyNull,   Constant, Null,   NotNull,  
> BotPTR,
> >   { /* AnyNull */ AnyNull,   AnyNull,   Constant, BotPTR, NotNull,  
> BotPTR,},

Which means that the comment should be:

>>      //  Object  |   TOP    |   AnyNull | Constant |   NotNull |   
>> BOTTOM   |

>>      //  AnyNull |....... U-AN .........|  U-Con   |    U-NN   |    
>> U-Bot   |

The AnyNull case never returns unloaded unchanged, except for the AN/ 
AN case, and having the word unloaded in the table implies that it  
simply returns it.

My comment about the Constant line can be ignored too since it ignores  
the results of the meet_ptr.  I might like the code better is the  
meet_ptr call was moved to it's uses since it's commonly completely  
ignored.

tom

>
>
> except TOP which corresponds to the special case in the code table  
> and NULL which
> is not in the table.
>
> Vladimir
>
> Tom Rodriguez wrote:
>> There's a nice comment above this code and it would be nice if the  
>> table and code agreed in both order and results.
>>      // Meet unloaded class with java/lang/Object
>>      //
>>      // Meet
>>      //          |                     Unloaded Class
>>      //  Object  |   TOP    |   AnyNull | Constant |   NotNull |   
>> BOTTOM   |
>>      //   
>> ===================================================================
>>      //   TOP     
>> | ..........................Unloaded......................|
>>      //  AnyNull |  U-AN     
>> |................Unloaded......................|
>>      // Constant | ... O-NN .................................. |    
>> O-BOT   |
>>      //  NotNull | ... O-NN .................................. |    
>> O-BOT   |
>>      //  BOTTOM  | ........................Object- 
>> BOTTOM ..................|
>> Your changes appear to make this line:
>>      //  AnyNull |  U-AN     
>> |................Unloaded......................|
>> become
>>      //  AnyNull |...........................  U- 
>> AN  ......................|
>> The -AN part isn't quite true since we're returning the results of  
>> meet_ptr which uses the table below.  AnyNull meeting NotNull  
>> returns NotNull.  The Constant line in the comments seems to be  
>> wrong too.
>> const TypePtr::PTR TypePtr::ptr_meet[TypePtr::lastPTR] 
>> [TypePtr::lastPTR] = {
>>  //              TopPTR,    AnyNull,   Constant, Null,   NotNull,  
>> BotPTR,
>>  { /* Top     */ TopPTR,    AnyNull,   Constant, Null,   NotNull,  
>> BotPTR,},
>>  { /* AnyNull */ AnyNull,   AnyNull,   Constant, BotPTR, NotNull,  
>> BotPTR,},
>>  { /* Constant*/ Constant,  Constant,  Constant, BotPTR, NotNull,  
>> BotPTR,},
>>  { /* Null    */ Null,      BotPTR,    BotPTR,   Null,   BotPTR,   
>> BotPTR,},
>>  { /* NotNull */ NotNull,   NotNull,   NotNull,  BotPTR, NotNull,  
>> BotPTR,},
>>  { /* BotPTR  */ BotPTR,    BotPTR,    BotPTR,   BotPTR, BotPTR,   
>> BotPTR,}
>> };
>> The code itself looks right to me after much rereading of the meet  
>> logic.
>> tom
>> On Mar 20, 2009, at 11:31 PM, Vladimir Kozlov wrote:
>>>
>>> http://cr.openjdk.java.net/~kvn/6820514/webrev.00
>>>
>>> Fixed 6820514: meet not symmetric failure in ctw
>>>
>>> Problem:
>>> Missing instance_id meet for the case j.l.Object:NotNull meets
>>> unloaded instance klass.
>>>
>>> Solution:
>>> Add missing code.
>>>
>>> Reviewed by:
>>>
>>> Fix verified (y/n): y, ctw test from bug
>>>
>>> Other testing:
>>> JPRT
>>>