intrinsic that makes runtime choice whether to revert to java

[Deneau, Tom]

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?

-- 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().
>>
>
>