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

[Christian Tornqvist]

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