RFR (S): 8154473: Update for CompilerDirectives to control stub generation and intrinsics

Vladimir Kozlov vladimir.kozlov at oracle.com
Wed May 18 19:40:32 UTC 2016


Okay, convert it to bug saying that -XX:DisableIntrinsic does not 
disable intrinsic in Interpreter. Which can lead to results 
inconsistencies since different code is used in compiled code and 
Interpreter.

Thanks,
Vladimir

On 5/18/16 12:36 PM, Deshpande, Vivek R wrote:
> HI Vladimir
>
> We can convert it to bug.
> With the change, using -XX:DisableIntrinsic=_dexp, will not generate the LIBM stub and all the calls will point to SharedRuntime::dexp correctly with stack adjustment.
> Currently with -XX:DisableIntrinsic=_dexp, stub gets generated and points to LIBM stub in interpreter.
> and just using option UseLibmIntrinsic to disable LIBM usage fails without stack adjustment for 64 bit.
> I can make changes for 32 bit in updated webrev.
>
> Regards,
> Vivek
>
>
> -----Original Message-----
> From: Vladimir Kozlov [mailto:vladimir.kozlov at oracle.com]
> Sent: Wednesday, May 18, 2016 12:24 PM
> To: Deshpande, Vivek R
> Cc: Viswanathan, Sandhya; hotspot compiler; hotspot-runtime-dev at openjdk.java.net runtime
> Subject: Re: RFR (S): 8154473: Update for CompilerDirectives to control stub generation and intrinsics
>
> First, 8154473 is RFE and we are post FC (feature complete). We have to wait until the process of RFEs approval is finalized.
>
> Or we can convert it to bug. Can you say what happens with current code if -XX:DisableIntrinsic=_dexp is used, for example? And what will happen after these changes?
>
> I don't see changes for:
> templateInterpreterGenerator_x86_32.cpp
>
> It would be better to have similar code there even if 32-bit code does not have stack problem.
>
>
> You did not implemented what Chris was asked:
>
>  > a)  Add a method in MacroAssembler to call
> MacroAssembler::call_VM_leaf_base (what you did) or
>  >
>  >b)  Add InterpreterMacroAssembler::call_VM_leaf and change
> MacroAssembler::call_VM_leaf to do:
>  >
>  > void MacroAssembler::call_VM_leaf(address entry_point, int
> number_of_arguments) {
>  >   MacroAssembler::call_VM_leaf_base(entry_point, number_of_arguments);
>  > }
>
> I would suggest an other method
>
> // Use this method when MacroAssembler version of call_VM_leaf_base()
> should be called.
> void MacroAssembler::call_VM_leaf0(address entry_point) {
>    MacroAssembler::call_VM_leaf_base(entry_point, 0);
> }
>
> Thanks,
> Vladimir
>
> On 5/18/16 11:27 AM, Deshpande, Vivek R wrote:
>> Hi Vladimir
>>
>> The latest webrev is here:
>> http://cr.openjdk.java.net/~vdeshpande/CompilerDirectives/8154473/webrev.03/
>> The bug ID for the same is:
>> https://bugs.openjdk.java.net/browse/JDK-8154473
>>
>> Thanks for looking into it.
>>
>> Regards,
>> Vivek
>>
>> -----Original Message-----
>> From: Vladimir Kozlov [mailto:vladimir.kozlov at oracle.com]
>> Sent: Wednesday, May 18, 2016 11:04 AM
>> To: Deshpande, Vivek R
>> Cc: Viswanathan, Sandhya; hotspot compiler; hotspot-runtime-dev at openjdk.java.net runtime
>> Subject: Re: RFR (S): 8154473: Update for CompilerDirectives to control stub generation and intrinsics
>>
>> Hi Vivek,
>>
>> Can you send link to latest webrev? The thread is long and it is not clear which webrev is final.
>>
>> Thanks,
>> Vladimir
>>
>> On 5/18/16 10:36 AM, Deshpande, Vivek R wrote:
>>>
>>>
>>> Hi Vladimir
>>>
>>>
>>>
>>> Could you please review and sponsor this patch with the current solution.
>>>
>>> This patch calls correct fallback version for LIBM methods when LIBM
>>> is disabled.
>>>
>>>
>>>
>>> Regards,
>>>
>>> Vivek
>>>
>>>
>>>
>>>
>>>
>>>
>>>
>>> *From:*Deshpande, Vivek R
>>> *Sent:* Tuesday, May 10, 2016 11:00 AM
>>> *To:* 'Christian Thalinger'
>>> *Cc:* Viswanathan, Sandhya; 'Vladimir Kozlov'; hotspot compiler;
>>> hotspot-runtime-dev at openjdk.java.net runtime
>>> *Subject:* RE: RFR (S): 8154473: Update for CompilerDirectives to
>>> control stub generation and intrinsics
>>>
>>>
>>>
>>> HI Christian
>>>
>>>
>>>
>>> We have not heard from runtime team regarding this change.
>>>
>>> Shall we go ahead with the current solution ?
>>>
>>> I can send out the latest webrev.
>>>
>>> Let me know your thoughts.
>>>
>>>
>>>
>>> Regards,
>>>
>>> Vivek
>>>
>>>
>>>
>>> *From:*Christian Thalinger [mailto:christian.thalinger at oracle.com]
>>> *Sent:* Monday, May 02, 2016 3:44 PM
>>> *To:* Deshpande, Vivek R
>>> *Cc:* Viswanathan, Sandhya; hotspot compiler;
>>> hotspot-runtime-dev at openjdk.java.net
>>> <mailto:hotspot-runtime-dev at openjdk.java.net> runtime
>>> *Subject:* Re: RFR (S): 8154473: Update for CompilerDirectives to
>>> control stub generation and intrinsics
>>>
>>>
>>>
>>>
>>>
>>>     On May 2, 2016, at 11:53 AM, Deshpande, Vivek R
>>>     <vivek.r.deshpande at intel.com <mailto:vivek.r.deshpande at intel.com>>
>>>     wrote:
>>>
>>>
>>>
>>>     Hi Christian
>>>
>>>
>>>
>>>     I had tried using call_VM_leaf() which you mentioned.
>>>
>>>
>>>
>>>     But this function
>>>
>>>
>>>
>>>     void MacroAssembler::call_VM_leaf(address entry_point, int number_of_arguments)
>>>     {
>>>
>>>       call_VM_leaf_base(entry_point, number_of_arguments);
>>>
>>>     }
>>>
>>>
>>>
>>>     ends up calling
>>>
>>>
>>>
>>>     void InterpreterMacroAssembler::call_VM_leaf_base(address
>>> entry_point,
>>>
>>>                                                       int number_of_arguments)
>>>     { ...
>>>
>>>     from interp_masm_x86.cpp instead of
>>>
>>>
>>>
>>>     void MacroAssembler::call_VM_leaf_base(address entry_point, int num_args)
>>>     { …
>>>
>>>
>>>
>>> Frankly, I didn’t know that there is an overload for call_VM_leaf_base
>>> in InterpreterMacroAssembler.  So this means there are two options:
>>>
>>>
>>>
>>> a)  Add a method in MacroAssembler to call
>>> MacroAssembler::call_VM_leaf_base (what you did) or
>>>
>>>
>>>
>>> b)  Add InterpreterMacroAssembler::call_VM_leaf and change
>>> MacroAssembler::call_VM_leaf to do:
>>>
>>>
>>>
>>> voidMacroAssembler::call_VM_leaf(address entry_point, int
>>> number_of_arguments) {
>>>
>>>   MacroAssembler::call_VM_leaf_base(entry_point, number_of_arguments);
>>>
>>> }
>>>
>>>
>>>
>>> I will let the runtime team decide.
>>>
>>>
>>>
>>>
>>>
>>>     So I had put mathfunc() to call the masm version of call_VM_leaf_base().
>>>
>>>
>>>
>>>     Let me know what you think.
>>>
>>>
>>>
>>>     Thanks and regards,
>>>     Vivek
>>>
>>>
>>>
>>>     *From:* Christian Thalinger [mailto:christian.thalinger at oracle.com]
>>>     *Sent:* Monday, May 02, 2016 1:50 PM
>>>     *To:* Deshpande, Vivek R
>>>     *Cc:* hotspot-compiler-dev at openjdk.java.net
>>>     <mailto:hotspot-compiler-dev at openjdk.java.net>
>>>     *Subject:* Re: RFR (S): 8154473: Update for CompilerDirectives to
>>>     control stub generation and intrinsics
>>>
>>>
>>>
>>>
>>>
>>>         On Apr 26, 2016, at 8:53 PM, Deshpande, Vivek R
>>>         <vivek.r.deshpande at intel.com
>>>         <mailto:vivek.r.deshpande at intel.com>> wrote:
>>>
>>>
>>>
>>>         Hi Christian
>>>
>>>
>>>
>>>         I have updated the webrev and link for the same is here:
>>>
>>>
>>> http://cr.openjdk.java.net/~vdeshpande/CompilerDirectives/8154473/webr
>>> ev.03/
>>>
>>>         I am using mathfunc() to call the masm version of
>>>         call_VM_leaf_base() and not the InterpreterMacroAssembler version.
>>>
>>>
>>>
>>>     That’s better but, again, there is nothing math-y about this method:
>>>
>>>     ! void MacroAssembler::mathfunc(address runtime_entry) {
>>>
>>>         MacroAssembler::call_VM_leaf_base(runtime_entry, 0);
>>>
>>>       }
>>>
>>>     Also, there is this method:
>>>
>>>
>>>
>>>     void MacroAssembler::call_VM_leaf(address entry_point, int number_of_arguments)
>>>     {
>>>
>>>       call_VM_leaf_base(entry_point, number_of_arguments);
>>>
>>>     }
>>>
>>>
>>>
>>>     which has:
>>>
>>>
>>>
>>>       void call_VM_leaf(address entry_point,
>>>
>>>                         int number_of_arguments = 0);
>>>
>>>
>>>
>>>     Get rid of mathfunc completely and use call_VM_leaf directly.
>>>
>>>
>>>
>>>
>>>
>>>         Regards,
>>>
>>>         Vivek
>>>
>>>
>>>
>>>
>>>
>>>         *From:* Christian Thalinger [mailto:christian.thalinger at oracle.com]
>>>         *Sent:* Thursday, April 21, 2016 2:35 PM
>>>         *To:* Deshpande, Vivek R <vivek.r.deshpande at intel.com
>>>         <mailto:vivek.r.deshpande at intel.com>>
>>>         *Cc:* Nils Eliasson <nils.eliasson at oracle.com
>>>         <mailto:nils.eliasson at oracle.com>>; hotspot-compiler-dev at openjdk.java.net
>>>         <mailto:hotspot-compiler-dev at openjdk.java.net>; Vladimir Kozlov
>>>         <vladimir.kozlov at oracle.com <mailto:vladimir.kozlov at oracle.com>>
>>>         *Subject:* Re: RFR (S): 8154473: Update for CompilerDirectives
>>>         to control stub generation and intrinsics
>>>
>>>
>>>
>>>
>>>
>>>             On Apr 20, 2016, at 2:13 PM, Deshpande, Vivek R
>>>             <vivek.r.deshpande at intel.com
>>>             <mailto:vivek.r.deshpande at intel.com>> wrote:
>>>
>>>
>>>
>>>             Hi
>>>
>>>
>>>
>>>             The correct URL for the updated webrev is this.
>>>
>>>
>>> http://cr.openjdk.java.net/~vdeshpande/CompilerDirectives/8154473/webr
>>> ev.02/
>>>
>>>
>>>
>>>         +void MacroAssembler::mathfunc(address runtime_entry) {
>>>
>>>         I don’t like the name of this method.  Mainly because it’s only
>>>         aligning the stack (shouldn’t that happen somewhere else?) and
>>>         doing this 0x20 stack frame thing which I still don’t understand.
>>>
>>>
>>>
>>>         Right, this is the one I was thinking about:
>>>
>>>
>>>
>>>         void MacroAssembler::call_VM_leaf_base(address entry_point, int num_args)
>>>         {
>>>
>>>
>>>
>>>
>>>             Sorry for the spam.
>>>
>>>
>>>
>>>             Regards,
>>>
>>>             Vivek
>>>
>>>
>>>
>>>             *From:* Deshpande, Vivek R
>>>             *Sent:* Wednesday, April 20, 2016 5:10 PM
>>>             *To:* Deshpande, Vivek R; 'Nils Eliasson';
>>>             'hotspot-compiler-dev at openjdk.java.net
>>>             <mailto:hotspot-compiler-dev at openjdk.java.net>'
>>>             *Cc:* 'Vladimir Kozlov'; 'Volker Simonis'; 'Christian
>>>             Thalinger'; Viswanathan, Sandhya
>>>             *Subject:* RE: RFR (S): 8154473: Update for
>>>             CompilerDirectives to control stub generation and
>>> intrinsics
>>>
>>>
>>>
>>>             Sent out the wrong link by mistake.
>>>
>>>
>>>
>>>             updated webrev:
>>>             http://cr.openjdk.java.net/~vdeshpande/CompilerDirectives/8154473/webrev.02/
>>>
>>> <http://cr.openjdk.java.net/~vdeshpande/CompilerDirectives/8154473/web
>>> rev.01/>
>>>
>>>
>>>
>>>             Regards
>>>
>>>             Vivek
>>>
>>>
>>>
>>>
>>>
>>>             *From:* Deshpande, Vivek R
>>>             *Sent:* Wednesday, April 20, 2016 5:07 PM
>>>             *To:* 'Nils Eliasson'; hotspot-compiler-dev at openjdk.java.net
>>>             <mailto:hotspot-compiler-dev at openjdk.java.net>
>>>             *Cc:* Vladimir Kozlov; Volker Simonis; Christian Thalinger;
>>>             Viswanathan, Sandhya
>>>             *Subject:* RE: RFR (S): 8154473: Update for
>>>             CompilerDirectives to control stub generation and
>>> intrinsics
>>>
>>>
>>>
>>>             Hi Nils
>>>
>>>
>>>
>>>             I have updated the webrev with all the suggestions.
>>>
>>>             updated webrev:
>>>
>>> http://cr.openjdk.java.net/~vdeshpande/CompilerDirectives/8154473/webr
>>> ev.01/
>>>
>>>             Thanks for your comments and review.
>>>
>>>
>>>
>>>             @Vladimir,
>>>
>>>             I have taken care of all the comments. Would you please
>>>             review and sponsor the patch.
>>>
>>>
>>>
>>>             Thanks and regards,
>>>
>>>             Vivek
>>>
>>>
>>>
>>>             *From:* Nils Eliasson [mailto:nils.eliasson at oracle.com]
>>>             *Sent:* Wednesday, April 20, 2016 12:27 PM
>>>             *To:* Deshpande, Vivek
>>>             R; hotspot-compiler-dev at openjdk.java.net
>>>             <mailto:hotspot-compiler-dev at openjdk.java.net>
>>>             *Cc:* Vladimir Kozlov; Volker Simonis; Christian Thalinger;
>>>             Viswanathan, Sandhya
>>>             *Subject:* Re: RFR (S): 8154473: Update for
>>>             CompilerDirectives to control stub generation and
>>> intrinsics
>>>
>>>
>>>
>>>             In vmSymbols.cpp together with the other flag checks.
>>>
>>>             Regards,
>>>             Nils
>>>
>>>             On 2016-04-20 02:44, Deshpande, Vivek R wrote:
>>>
>>>                 HI Nils
>>>
>>>
>>>
>>>                 Yes you are right the function accesses the command line
>>>                 flag DisableIntrinsic and changes are static.
>>>
>>>                 Could you point me the right location for the function ?
>>>
>>>                 Also I have updated the webrev with rest of the comments
>>>                 here:
>>>
>>>
>>> http://cr.openjdk.java.net/~vdeshpande/CompilerDirectives/8154473/webr
>>> ev.01/
>>>
>>>
>>>
>>>                 Regards,
>>>
>>>                 Vivek
>>>
>>>
>>>
>>>                 *From:* hotspot-compiler-dev
>>>                 [mailto:hotspot-compiler-dev-bounces at openjdk.java.net]*On Behalf
>>>                 Of *Nils Eliasson
>>>                 *Sent:* Tuesday, April 19, 2016 5:55 AM
>>>                 *To:* hotspot-compiler-dev at openjdk.java.net
>>>                 <mailto:hotspot-compiler-dev at openjdk.java.net>
>>>                 *Subject:* Re: RFR (S): 8154473: Update for
>>>                 CompilerDirectives to control stub generation and
>>> intrinsics
>>>
>>>
>>>
>>>                 Hi Vivek,
>>>
>>>                 The changes in is_intrinsic_disabled in
>>>                 compilerDirectives.* are static and only access the
>>>                 command line flag DisableIntrinsics. As long as stubs
>>>                 are only generated during startup and don't have a
>>>                 method context - that is ok - but it doesn't belong in
>>>                 the compilerDirectives-files if it doens't use directives.
>>>
>>>                 Regards,
>>>                 Nils
>>>
>>>                 On 2016-04-18 19:38, Deshpande, Vivek R wrote:
>>>
>>>                     Hi all
>>>
>>>
>>>
>>>                     I would like to contribute a patch which helps to
>>>                     control the intrinsics in interpreter, c1 and c2 by
>>>                     disabling the stub generation.
>>>
>>>                     This uses -XX:DisableIntrinsic option to achieve the
>>>                     same.
>>>
>>>                     Could you please review and sponsor this patch.
>>>
>>>
>>>
>>>                     Bug-id:
>>>
>>>                     https://bugs.openjdk.java.net/browse/JDK-8154473
>>>                     webrev:
>>>
>>>                     http://cr.openjdk.java.net/~vdeshpande/CompilerDirectives/8154473/webrev.00/
>>>
>>> <http://cr.openjdk.java.net/%7Evdeshpande/CompilerDirectives/8154473/w
>>> ebrev.00/>
>>>
>>>
>>>
>>>                     Thanks and regards,
>>>
>>>                     Vivek
>>>
>>>
>>>


More information about the hotspot-runtime-dev mailing list