intrinsic that makes runtime choice whether to revert to java

[Deneau, Tom]

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.

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.

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?

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