intrinsic that makes runtime choice whether to revert to java

[Vladimir Kozlov]

Deneau, Tom wrote:
> Vladimir --
> 
> Thanks for the explanation. Just trying to make sure I understand this...
> 
> I think you're saying that when the C2 compiler decided to compile this
> particular invocation of CipherBlockChaining.encrypt, it "knows" that
> the embeddedCipher will be AESCrypt type, so the instanceof check
> I am doing at runtime will always be true.

Yes, C2 does CHA (class hierarchy analysis) to find and use normal class instead 
of abstract. There is debug flag -XX:-UseUniqueSubclasses to switch off this 
optimization.

> 
> But if I had a test case that switched back and forth between say DESCrypt and AESCrypt, then
> when we compiled that particular invocation, we would get a real conditional in the predicated path.

Correct, you need to use DESCrypt at least once (so it is loaded).

> 
> I think we're also saying here that if by the time something gets
> hot enough to compile and AESCrypt class has never even been loaded, we should
> just forget about ever taking the intrinsic for this invocation and always
> take the java path.  And if later it turns out that later AESCrypt does
> actually get loaded, maybe things will get recompiled and if not it will
> still be functionally correct?

Correct, C2 uses dependencies mechanism to track class hierarchy changes and 
recompile methods for which such dependencies are invalidated. See code in 
TypeOopPtr::make_from_klass_common(). But it does not trace if particular class 
is loaded later. The only case when it will be recompiled if you had only one 
not AESCrypt class and the method is inlined. With many classes we generate 
virtual call so it will work when AESCrypt is loaded.

Vladimir

> 
> -- Tom
> 
> 
> 
> 
> 
> -----Original Message-----
> From: Vladimir Kozlov [mailto:vladimir.kozlov at oracle.com] 
> Sent: Wednesday, July 11, 2012 11:35 PM
> To: Deneau, Tom
> Subject: Re: intrinsic that makes runtime choice whether to revert to java
> 
> C2 is very smart :)
> 
> It uses unique subclass instead of abstract class. In your case  AESCrypt is 
> unique implementor of SymmetricCipher abstract class and that is why the 
> predicate is always true in your case:
> 
> [t at 40 l at 40]: print field_klass->print()
> <ciInstanceKlass name=com/sun/crypto/provider/SymmetricCipher loader=0xc5432720 
> loaded=true initialized=true finalized=false subklass=true size=17 
> flags=DEFAULT_ACCESS,super,abstract ident=762 PERM address=0x715a728>
> 
>   5617     const Type *type = TypeOopPtr::make_from_klass(field_klass->as_klass());
> 
> [t at 40 l at 40]: next
> [t at 40 l at 40]: print type->dump()
> com/sun/crypto/provider/AESCrypt:exact *
> 
> You don't need Region node. Use next code for dead fast path - then 
> kit.stopped() will be true:
> 
>    if (!klass_AESCrypt->is_loaded()) {
>      set_control(top());     // no regular fast path
>      return NULL;
>    }
> 
> Your new check (slow_ctl != NULL) in generate() is correct.
> 
> Also change flag methods in class PredictedIntrinsicGenerator:
> 
> < virtual bool      is_virtual()   const    { return _intrinsic->is_virtual(); }
> < virtual bool      is_inlined()   const    { return true; }
> --
>  > virtual bool      is_virtual()   const    { return true; }
>  > virtual bool      is_inline()    const    { return true; }
> 
> Vladimir
> 
> Deneau, Tom wrote:
>> Vladimir --
>>
>>  
>>
>> Ah, that's good to hear.  I hope you had a good vacation..
>>
>>  
>>
>> I made a few changes so things are not crashing now but I still don't think
>>
>> it is really correct.  The webrev at 
>> http://cr.openjdk.java.net/~tdeneau/predicated-problem/webrev/ has been 
>> updated accordingly.
>>
>>  
>>
>> The changes I made were:
>>
>>  
>>
>> 1) As mentioned before, the code that is generating the predicate (shown 
>> here) is
>>
>>    always creating a NULL node from generate_guard, which as I 
>> understand it means the instanceof conditional
>>
>>    will never be false?  This doesn't seem right.
>>
>>  
>>
>>    I think we wanted to reserve a NULL node to mean we could not 
>> generate the predicate.
>>
>>    So I constructed a Region node with the single NULL node as input, 
>> which again doesn't
>>
>>    seem right but at least has a non-NULL node to return.
>>
>>  
>>
>>   ciKlass* klass_AESCrypt = tinst->klass()->as_instance_klass()
>>
>>                  
>> ->find_klass(ciSymbol::make("com/sun/crypto/provider/AESCrypt"));
>>
>>   if (!klass_AESCrypt->is_loaded()) return NULL;
>>
>>  
>>
>>   ciInstanceKlass* instklass_AESCrypt = klass_AESCrypt->as_instance_klass();
>>
>>  
>>
>>   _sp += nargs;          // gen_instanceof might do an uncommon trap
>>
>>   Node* instof = gen_instanceof(embeddedCipherObj, 
>> makecon(TypeKlassPtr::make(instklass_AESCrypt)));
>>
>>   _sp -= nargs;
>>
>>   Node* cmp_instof  = _gvn.transform(new (C, 3) CmpINode(instof, 
>> intcon(1)));
>>
>>   Node* bool_instof  = _gvn.transform(new (C, 2) BoolNode(cmp_instof, 
>> BoolTest::ne));
>>
>>  
>>
>>   Node* instof_false = generate_guard(bool_instof, NULL, PROB_MIN);
>>
>>   //instanceOf == true, fallthrough
>>
>>  
>>
>>  
>>
>> // I added the following so we wouldn't return a NULL node.
>>
>>    RegionNode* region = new(C, 2) RegionNode(2);
>>
>>    region->init_req(1, instof_false);
>>
>>    record_for_igvn(region);
>>
>>    return _gvn.transform(region);
>>
>>  
>