MVT-based vectors: first stab

[Vladimir Ivanov]

> thanks a lot for the nice summary! I think it would makes sense to file enhancements/bugs for the individual issues.

FYI here's the list of issues:

RFEs
     https://bugs.openjdk.java.net/browse/JDK-8183130
         core-libs/j.l.i [MVT] Implement intrinsics for Q-typed LambdaForms

     https://bugs.openjdk.java.net/browse/JDK-8183137
         hotspot/compiler [MVT] C2 doesn't eliminate redundant value 
type allocations inside loops

Bugs
     https://bugs.openjdk.java.net/browse/JDK-8183129
         hotspot/compiler [MVT] Type::get_typeflow_type() doesn't 
distinguish between TypeValueTypePtr & TypeValueType

     https://bugs.openjdk.java.net/browse/JDK-8183131
         core-libs/j.l.i [MVT] 2-slot type (long & double) support is 
broken in LambdaFormBuilder

     https://bugs.openjdk.java.net/browse/JDK-8183132
         core-libs/j.l.i [MVT] MHI.unboxResultHandle misses unboxing 
step for Q-types

     https://bugs.openjdk.java.net/browse/JDK-8183133
         core-libs/j.l.i [MVT] Arraylength method handle on Q-typed 
array shouldn't use ArrayAccessor.lengthL(Object[])

     https://bugs.openjdk.java.net/browse/JDK-8183135
         core-libs/j.l.i [MVT] Code builder doesn't fully support 
constant pool patching

     https://bugs.openjdk.java.net/browse/JDK-8183138
         core-libs/j.l.i Code Builder: Problems with handling complex 
control flow

Best regards,
Vladimir Ivanov


> On 28.06.2017 15:17, Vladimir Ivanov wrote:
>> I hit some bugs along the way
> 
> I had a closer look and (as you already figured out) one problem is that typeflow analysis incorrectly sets the type of a PhiNode to TypeValueType instead of TypeValueTypePtr for __Value. When inlining a method handle intrinsic in CallGenerator::for_method_handle_inline() we should cast all arguments to their actual types but we miss the PhiNode because it's already of TypeValueType:
> 
> t: <ciValueKlass name=valhalla/vector/Long2$Value loader=0x00000006d8776320 loaded=true initialized=true finalized=false subklass=false size=32 flags=public,final,super super=java/lang/__Value ident=1108 address=0x00007f218008e270>
> arg_type: valuetype[0]:{empty}
> sig_type: valuetype* valhalla/vector/Long2$Value:NotNull
> arg: 659	Phi	===  652 _  626  [[ 616  662  678  680  684  730  760  785  790  793  837  867  874  889  891  894 ]]  #valuetype[0]:{empty} !jvms: 1310540333::invoke @ bci:170 858242339::invokeExact_MT @ bci:19 VectorTest::acc @ bci:4 VectorTest::main @ bci:69
> 
> We then fail in Parse::do_vunbox because the input node is a Phi (and not a ValueTypeNode).
> 
> I think in addition to the fix in type.cpp, we should add this assert to catch such problems in the future:
> 
> --- old/src/share/vm/opto/callGenerator.cpp	2017-06-28 15:25:40.812039870 +0200
> +++ new/src/share/vm/opto/callGenerator.cpp	2017-06-28 15:25:40.720039875 +0200
> @@ -896,10 +896,11 @@
>     if (t->is_valuetype()) {
>       assert(!(arg_type->isa_valuetype() && t == kit.C->env()->___Value_klass()), "need a pointer to the value type");
>       if (arg_type->isa_valuetypeptr() && t != kit.C->env()->___Value_klass()) {
> -      const Type* sig_type = TypeOopPtr::make_from_klass(t->as_klass());
>         Node* cast = gvn.transform(new CheckCastPPNode(kit.control(), arg, sig_type));
>         Node* vt = ValueTypeNode::make(gvn, kit.merged_memory(), cast);
>         kit.set_argument(arg_nb, vt);
> +    } else {
> +      assert(t == kit.C->env()->___Value_klass() || arg->is_ValueType(), "inconsistent argument");
>       }
>     } else {
>       if (arg_type->isa_oopptr() && !arg_type->higher_equal(sig_type)) {
> 
> I'll take the chance to look at the method handle implementation in a bit more detail and try to come up with a regression test for this to extend our ValueTypesTestBench.
> 
> Best regards,
> Tobias
>