intrinsic that makes runtime choice whether to revert to java

[Vladimir Kozlov]

Deneau, Tom wrote:
> Krystal --
> 
> Vladimir added the ability for an intrinsic to execute some predicated code and then
> if the predication is false to take the regular java code path instead of the instrinsic
> code path.
> 
> In my predicated check path, I am doing the same instanceof check which I commented out below.
> So if I've made it to the intrinsic setup code, I don't understand why I have to do it again.
> Or does the second check actually get optimized away by the compiler?

Yes, you don't need instanceof check in inlinine_foo() method if it is present 
in inline_foo_predicate(). I simple used your example 3) code for predicate to 
show how to do that.

And, please, remove next line from my example which I put by mistake. The 
embeddedCipherObj value is loaded by Load node generated in intrinsic code and 
it is not taking from expression stack.

     _gvn.replace_in_map(embeddedCipherObj, cast);

Vladimir

> 
> -- Tom
> 
> 
> 
> 
> From: Krystal Mok [mailto:rednaxelafx at gmail.com] 
> Sent: Thursday, July 05, 2012 6:04 PM
> To: Deneau, Tom
> Cc: Vladimir Kozlov; hotspot-compiler-dev at openjdk.java.net
> Subject: Re: intrinsic that makes runtime choice whether to revert to java
> 
> Hi Tom,
> 
> I think you'd still need an explicit subtype check. See the example in Parse::catch_inline_exceptions():
> 
>     // Check the type of the exception against the catch type
>     const TypeKlassPtr *tk = TypeKlassPtr::make(klass);
>     Node* con = _gvn.makecon(tk);
>     Node* not_subtype_ctrl = gen_subtype_check(ex_klass_node, con);
>     if (!stopped()) {
>       PreserveJVMState pjvms(this);
>       const TypeInstPtr* tinst = TypeOopPtr::make_from_klass_unique(klass)->cast_to_ptr_type(TypePtr::NotNull)->is_instptr();
>       assert(klass->has_subklass() || tinst->klass_is_exact(), "lost exactness");
>       Node* ex_oop = _gvn.transform(new (C, 2) CheckCastPPNode(control(), ex_node, tinst));
>       push_ex_oop(ex_oop);      // Push exception oop for handler
> 
> The CheckCastPP node is only meant to tell later parts of the graph that "we're sure we're getting the expected type on this path", instead of the actually checking logic.
> 
> - Kris
> 
> On Fri, Jul 6, 2012 at 6:50 AM, Deneau, Tom <tom.deneau at amd.com> wrote:
> Vladimir --
> 
> Limiting here to your reponse to my second question...
> The code you show for casting, I assume I can safely eliminate the instanceOf check
> (the lines commented out below)
> because we already checked for this in the predicated code path. so we
> would not have gotten to this intrinsic setup code if the runtime object were the wrong type.
> 
> -- Tom
> 
> 
>    const TypeKlassPtr* aklass = TypeKlassPtr::make(klass_AESCrypt);
> //   _sp += nargs;          // gen_instanceof might do an uncommon trap
> //   Node* instof = gen_instanceof(embeddedCipherObj, makecon(aklass));
> //   _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);
>    // fall through if the instance exactly matches the desired type
>    const TypeOopPtr* xtype = aklass->as_instance_type();
>    Node* cast = new(C, 2) CheckCastPPNode(control(), embeddedCipherObj, xtype);
>    cast = _gvn.transform(cast);
>    _gvn.replace_in_map(embeddedCipherObj, cast);
>    Node* k_len = get_key_len_from_aescrypt_object(cast);
> 
> -----Original Message-----
> From: Vladimir Kozlov [mailto:vladimir.kozlov at oracle.com]
> Sent: Thursday, July 05, 2012 4:55 PM
> To: Deneau, Tom
> Cc: hotspot-compiler-dev at openjdk.java.net
> Subject: Re: intrinsic that makes runtime choice whether to revert to java
> 
> Deneau, Tom wrote:
>> Vladimir --
>>
>> The code you provided helps a lot, thank you.
>> I am still testing it, but it seems to work for the basic cases.
>>
>> A couple of questions based on my not having done much with GraphKit:
>>
>>    1) I want to do a test at compile time (or maybe even earlier at
>>       stub generation time) as to whether a certain library class is a
>>       version that has a certain field.  This would fail if are
>>       mistakenly using the old library with a new JVM, which would
>>       indicate we cannot use the intrinsics.  I suspect this is done
>>       using the class from the systemDictionary but I'm not sure.
> 
> Look on usage of find_local_field() method in thread.cpp and other places.
> 
>>    2) At compile time, when generating the code for the fast path, I
>>       want to generate a Node* to load a certain field from the
>>       embeddedCipher object (since we have made it past the predicate,
>>       we know that the embeddedCipher object is an instance of the
>>       class we expect).  I need to do this so that I can pass the
>>       field to the stub.  But at compile time, the particular instance
>>       that triggered compilation might not have an embeddedCipher
>>       object of the proper type.  Still I want to generate a Node*
>>       that will load a field from the embeddedCipher object as if it
>>       were the proper type.  And I am not sure how to do that.  (My
>>       current code failes to generate the intrinsic unless the
>>       particular instance that triggered compilation is the correct
>>       type).  I hope this question makes sense.
> 
> I assume by the "fast path" you mean the intrinsic for whole encrypt() method.
> Right? You need to cast field object's type to AESCrypt if I understand your
> question correctly:
> 
>    const TypeKlassPtr* aklass = TypeKlassPtr::make(klass_AESCrypt);
>    _sp += nargs;          // gen_instanceof might do an uncommon trap
>    Node* instof = gen_instanceof(embeddedCipherObj, makecon(aklass));
>    _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);
>    // fall through if the instance exactly matches the desired type
>    const TypeOopPtr* xtype = aklass->as_instance_type();
>    Node* cast = new(C, 2) CheckCastPPNode(control(), embeddedCipherObj, xtype);
>    cast = _gvn.transform(cast);
>    _gvn.replace_in_map(embeddedCipherObj, cast);
>    Node* k_len = get_key_len_from_aescrypt_object(cast);
> 
>>    3) If I want my predicate to be based on two Boolean tests,
>>       similar to the following java code
>>
>>       if ((obj instanceof XXXClass) && ((XXXClass)(obj).field == SomeConstant))
>>
>>       and I want to of course skip the second test if the first test fails.
>>       Is it something like the following?
> 
> You need Region node to merge 2 false paths:
> 
>    RegionNode* region = new(C, 3) RegionNode(3);
> ...
>    region->init_req(1, instof_false);
> ...
>    region->init_req(2, klen_false);
>    record_for_igvn(region);
>    return _gvn.transform(region);
> 
>>   _sp += nargs;          // gen_instanceof might do an uncommon trap
>>   Node* instof = gen_instanceof(embeddedCipherObj, makecon(TypeKlassPtr::make(klass_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
>>
>>   Node* k_len = get_key_len_from_aescrypt_object(embeddedCipherObj);
>>   Node* cmp_klen  = _gvn.transform(new (C, 3) CmpINode(k_len, intcon(44)));   // corresponds to keys we can handle
>>   Node* bool_klen  = _gvn.transform(new (C, 2) BoolNode(cmp_klen, BoolTest::ne));
> 
> Why exact 44 and not <= ?
> 
> Vladimir
> 
>>   Node* klen_false = generate_guard(bool_klen, NULL, PROB_MIN);
>>   return klen_false;
>>
>> -- Tom
>>
>> -----Original Message-----
>> From: Vladimir Kozlov [mailto:vladimir.kozlov at oracle.com]
>> Sent: Monday, July 02, 2012 9:57 PM
>> To: Deneau, Tom
>> Cc: hotspot-compiler-dev at openjdk.java.net
>> Subject: Re: intrinsic that makes runtime choice whether to revert to java
>>
>> I finally had time today to look on this. Here is code:
>>
>> http://cr.openjdk.java.net/~kvn/pred_intr/webrev
>>
>> But I did not test or optimize it (part of new code in callGenerator.cpp should
>> be shared with PredictedCallGenerator from which it was cloned).
>>
>> Vladimir
>>
>> Deneau, Tom wrote:
>>> Hi all --
>>>
>>> I am writing a hotspot intrinsic but I want it to make a runtime
>>> choice whether to execute the intrinsic code or to execute the regular
>>> java code.
>>>
>>> I got the suggestion below from Vladimir but I am not experienced
>>> enough with the graphkit functionality to see how to do this.  Would
>>> someone be able to provide the code skeleton for this?  I can generate
>>> the region to actually do my intrinsic and I believe I can generate the
>>> BoolNode which determines which path to take.
>>>
>>> The other choice would be to modify the actual JDK library routine to
>>> do the runtime tests and call a new routine which the intrinsic would
>>> hook into.  I have done this for testing but I am assuming this is more
>>> intrusive since on some architectures the intrinsic will not even be generated.
>>>
>>> -- Tom Deneau
>>>
>>>> We don't have such mechanism currently. But it is not difficult to implement.
>>>> Look on the code in doCall.cpp and callGenerator files. Instead of calling
>>>> find_intrinsic() you need to do something special for your case. Like code for
>>>> CallGenerator::for_method_handle_call() where you will generate class check and
>>>> on one path you will get intrinsic (by calling find_intrinsic()) and on other
>>>> path is regular CallGenerator::for_inline().
>>
> 
> 
>