RFR: 8164358: [JVMCI] expose Hotspot intrinsics and HotSpotIntrinsicCandidate info to JVMCI compilers

[Vladimir Kozlov]

I think it should stay as bug.

 From my POV it is missing expected functionality - it should behave the same way as existing native JITs.
Imaging that you specify intrinsic flags but you will get different results - it is bug.
Also it is not new functionality - it exist with native JITs already.

Thanks,
Vladimir

On 8/19/16 8:59 AM, Doug Simon wrote:
> Hi Christian,
>
> I’ve reformulated the description to clarify why the absence of these features is a bug. I’ll keep this in mind for future.
>
> -Doug
>
>> On 19 Aug 2016, at 17:41, Christian Tornqvist <christian.tornqvist at oracle.com> wrote:
>>
>> This issue looks like an RFE and not a bug though?
>>
>> Thanks,
>> Christian
>>
>> -----Original Message-----
>> From: hotspot-compiler-dev [mailto:hotspot-compiler-dev-bounces at openjdk.java.net] On Behalf Of Dmitrij Pochepko
>> Sent: Friday, August 19, 2016 11:38 AM
>> To: Doug Simon <doug.simon at oracle.com>
>> Cc: hotspot compiler <hotspot-compiler-dev at openjdk.java.net>; Christian Thalinger <cthalinger at twitter.com>
>> Subject: Re: RFR: 8164358: [JVMCI] expose Hotspot intrinsics and HotSpotIntrinsicCandidate info to JVMCI compilers
>>
>> Hi,
>>
>> What about tests? Seems like this functionality is not covered by current tests. We might get it broken without noticing in case no tests exists.
>> I think at least some sanity test should be added.
>>
>> In case tests are added(or already exists) looks good to me (not a reviewer).
>>
>> Thanks,
>> Dmitrij
>> I changed it to only try reuse the declaringClass string (which has the most overlap): http://cr.openjdk.java.net/~dnsimon/8164358v2/ -Doug
>>>> On 18 Aug 2016, at 23:59, Doug Simon <doug.simon at oracle.com> wrote:
>>>>
>>>>
>>>>> On 18 Aug 2016, at 20:01, Christian Thalinger <cthalinger at twitter.com> wrote:
>>>>>
>>>>> src/jdk.vm.ci/share/classes/jdk.vm.ci.hotspot/src/jdk/vm/ci/hotspot/
>>>>> CompilerToVM.java
>>>>>
>>>>>      *         [String name, Long value, ...] vmAddresses,
>>>>>
>>>>> +     *         [String name, Long value, ...] vmAddresses,
>>>>>
>>>>>      *         VMFlag[] vmFlags
>>>>>
>>>>> +     *         VMIntrinsicMethod[] vmIntrinsics
>>>>> Looks like a copy-paste problem.
>>>> Oops! Will fix.
>>>>
>>>>> src/share/vm/jvmci/jvmciCompilerToVM.cpp
>>>>>
>>>>> Why do we have to do the prev_##var##_sid == sid dance?  I guess it’s some optimization but does it really matter?
>>>> It cuts down the number Strings allocated by about 35% (900 vs 582). With full string interning, the savings is around 55% (415) but the overhead of using a HashSet from VM code is prohibitive. I won’t argue strongly that it makes a big difference but since the one-element cache (per VMIntrinsicMethod string field) is fast and cheap, I don’t see why not to use it. The alternative code would look something like this:
>>>>
>>>>  objArrayHandle vmIntrinsics = oopFactory::new_objArray(VMIntrinsicMethod::klass(), (vmIntrinsics::ID_LIMIT - 1), CHECK_NULL);
>>>>  int index = 0;
>>>> #define VM_SYMBOL_TO_STRING(s) java_lang_String::create_from_symbol(vmSymbols::symbol_at(vmSymbols::VM_SYMBOL_ENUM_NAME(s)), THREAD)
>>>> #define VM_INTRINSIC_INFO(id, kls, name, sig, ignore_fcode) {                  \
>>>>    instanceHandle vmIntrinsicMethod = InstanceKlass::cast(VMIntrinsicMethod::klass())->allocate_instance_handle(CHECK_NULL); \
>>>>    Handle kls_string = VM_SYMBOL_TO_STRING(kls);                              \
>>>>    Handle name_string = VM_SYMBOL_TO_STRING(name);                            \
>>>>    Handle sig_string = VM_SYMBOL_TO_STRING(sig);                              \
>>>>    VMIntrinsicMethod::set_declaringClass(vmIntrinsicMethod, kls_string());    \
>>>>    VMIntrinsicMethod::set_name(vmIntrinsicMethod, name_string());             \
>>>>    VMIntrinsicMethod::set_descriptor(vmIntrinsicMethod, sig_string());        \
>>>>    VMIntrinsicMethod::set_id(vmIntrinsicMethod, vmIntrinsics::id);            \
>>>>    vmIntrinsics->obj_at_put(index++, vmIntrinsicMethod());                    \
>>>>  }
>>>>
>>>>  VM_INTRINSICS_DO(VM_INTRINSIC_INFO, VM_SYMBOL_IGNORE,
>>>> VM_SYMBOL_IGNORE, VM_SYMBOL_IGNORE, VM_ALIAS_IGNORE) #undef
>>>> VM_SYMBOL_TO_STRING #undef VM_INTRINSIC_INFO
>>>>
>>>> Better?
>>>>
>>>> -Doug
>>>>
>>>>> If we don’t have to do that I would prefer is that macro would be a method (or two).
>>>>>
>>>>>> On Aug 18, 2016, at 6:50 AM, Doug Simon <doug.simon at oracle.com> wrote:
>>>>>>
>>>>>> Please review this change which allows JVMCI compilers to see what intrinsics are defined by the VM and which methods have been annotated by HotSpotIntrisicCandidate.
>>>>>>
>>>>>> https://bugs.openjdk.java.net/browse/JDK-8164358
>>>>>> http://cr.openjdk.java.net/~dnsimon/8164358/
>>>>>>
>>>>>> Thanks,
>>>>>>
>>>>>> -Doug
>>
>>
>