[prev in list] [next in list] [prev in thread] [next in thread]
List: openjdk-hotspot-compiler-dev
Subject: Re: Copyright header question: 8132207: Update for x86 exp in the math lib
From: Igor Veresov <igor.veresov () oracle ! com>
Date: 2015-09-18 7:56:28
Message-ID: C966158B-372C-4D35-957C-E670E374F553 () oracle ! com
[Download RAW message or body]
Hi Vivek,
There are some slight build problems with this.
Probably a missling include of macroAssembler_x86.hpp. Try building without \
precompiled headers (USE_PRECOMPILED_HEADER=0).
/opt/jprt/T/P1/062856.iggy/s/hotspot/src/cpu/x86/vm/macroAssembler_x86_libm.cpp:192:6: \
error: incomplete type 'MacroAssembler' named in nested name specifier void \
MacroAssembler::fast_exp(XMMRegister xmm0, XMMRegister xmm1, XMMRegister xmm2, \
XMMRegister xmm3, XMMRegister xmm4, XMMRegister xmm5, XMMRegister xmm6, XMMRegister \
xmm7, Register eax, Register ecx, Register edx, Register tmp) { ^~~~~~~~~~~~~~~~
/opt/jprt/T/P1/062856.iggy/s/hotspot/src/share/vm/asm/assembler.hpp:40:7: note: \
forward declaration of 'MacroAssembler' class MacroAssembler;
^
1 error generated.
On non-intel platforms, this is an issue. supports_sse2() here should probably be \
abstracted away as "should_use_math_stub()" or something to make it more platform \
independent. Otherwise on other CPUs, supports_sse2() is simply \
undeclared.
/opt/jprt/T/P1/062856.iggy/s/hotspot/src/share/vm/opto/library_call.cpp
/opt/jprt/T/P1/062856.iggy/s/hotspot/src/share/vm/opto/library_call.cpp: In member \
function 'bool LibraryCallKit::inline_math_native(vmIntrinsics::ID)':
/opt/jprt/T/P1/062856.iggy/s/hotspot/src/share/vm/opto/library_call.cpp:1765:12: \
error: 'supports_sse2' is not a member of 'VM_Version'
return VM_Version::supports_sse2() ? runtime_math(OptoRuntime::Math_D_D_Type(), \
StubRoutines::dexp(), "dexp") : ^
Also, I believe you tested this, but could you please add a unit test (somewhere to \
hotspot/test/compiler) that would verify that j.l.Math.exp() returns results as \
intended? Otherwise there's some pretty complicated code there to be proved correct \
just by eyeballing it.
igor
> On Sep 16, 2015, at 2:54 PM, Deshpande, Vivek R <vivek.r.deshpande@intel.com> \
> wrote:
> Hi Igor,
>
> Could you please sponsor this patch. Vladimir and Christian have reviewed the \
> patch. Thanks.
>
> Regards,
> Vivek
>
> -----Original Message-----
> From: Christian Thalinger [mailto:christian.thalinger@oracle.com]
> Sent: Wednesday, September 16, 2015 2:05 PM
> To: Deshpande, Vivek R
> Cc: Vladimir Kozlov; Viswanathan, Sandhya; hotspot-compiler-dev@openjdk.java.net
> Subject: Re: Copyright header question: 8132207: Update for x86 exp in the math lib
>
> Looks much better. Thanks you.
>
> > On Sep 16, 2015, at 8:10 AM, Deshpande, Vivek R <vivek.r.deshpande@intel.com> \
> > wrote:
> > Hi Christian, Please let us know if the updated patch looks good to you.
> >
> > -----Original Message-----
> > From: Vladimir Kozlov [mailto:vladimir.kozlov@oracle.com]
> > Sent: Wednesday, September 16, 2015 10:58 AM
> > To: Deshpande, Vivek R; Viswanathan, Sandhya;
> > hotspot-compiler-dev@openjdk.java.net
> > Subject: Re: Copyright header question: 8132207: Update for x86 exp in
> > the math lib
> >
> > Looks good to me.
> >
> > Thanks,
> > Vladimir
> >
> > On 9/16/15 10:51 AM, Deshpande, Vivek R wrote:
> > > Hi Vladimir
> > >
> > > We have updated the patch with your suggestions and with the suggested \
> > > copyright header.
> > > Bug-id:
> > > https://bugs.openjdk.java.net/browse/JDK-8132207
> > > webrev:
> > > http://cr.openjdk.java.net/~mcberg/8132207/webrev.03/
> > >
> > > Thanks.
> > >
> > > Regards,
> > > Vivek
> > >
> > > -----Original Message-----
> > > From: Viswanathan, Sandhya
> > > Sent: Friday, September 11, 2015 3:32 PM
> > > To: Vladimir Kozlov; Deshpande, Vivek R; Christian Thalinger
> > > Cc: david.katleman@oracle.com; David Holmes
> > > Subject: RE: Copyright header question: 8132207: Update for x86 exp
> > > in the math lib
> > >
> > > Hi Vladimir,
> > >
> > > Yes we can do that. We will send you the updated patch.
> > >
> > > Thanks,
> > > Sandhya
> > >
> > >
> > > -----Original Message-----
> > > From: Vladimir Kozlov [mailto:vladimir.kozlov@oracle.com]
> > > Sent: Friday, September 11, 2015 3:00 PM
> > > To: Viswanathan, Sandhya; Deshpande, Vivek R; Christian Thalinger
> > > Cc: david.katleman@oracle.com; David Holmes
> > > Subject: Re: Copyright header question: 8132207: Update for x86 exp
> > > in the math lib
> > >
> > > Can you do as I suggested in previous mail?
> > >
> > > /*
> > > * Copyright (c) 2015, Intel Corporation.
> > > * Intel Math Library (LIBM) Source Code.
> > > * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
> > > *
> > > * This code is free software; you can redistribute it and/or
> > > modify it
> > >
> > > and the rest of standard Oracle template after that.
> > >
> > > Thanks,
> > > Vladimir
> > >
> > > On 9/11/15 2:50 PM, Viswanathan, Sandhya wrote:
> > > > Hi Vladimir,
> > > >
> > > > It has been a week since; please let us know how to proceed on the copyright \
> > > > header. Vivek has everything else done and ready to submit the updated patch.
> > > >
> > > > Best Regards,
> > > > Sandhya
> > > >
> > > > -----Original Message-----
> > > > From: Vladimir Kozlov [mailto:vladimir.kozlov@oracle.com]
> > > > Sent: Thursday, September 03, 2015 2:25 PM
> > > > To: Viswanathan, Sandhya; Deshpande, Vivek R; Christian Thalinger
> > > > Cc: david.katleman@oracle.com; David Holmes
> > > > Subject: Copyright header question: 8132207: Update for x86 exp in
> > > > the math lib
> > > >
> > > > Hi Davis K.
> > > >
> > > > I hope you can help or advise us about Copyright header in new file
> > > > contributed by Intel:
> > > >
> > > > http://cr.openjdk.java.net/~mcberg/8132207/webrev.01/raw_files/new/s
> > > > r c /cpu/x86/vm/macroAssembler_x86_libm.cpp
> > > >
> > > > Currently it has 2 copyright blocks, Oracle's and Intel's.
> > > > It looks like I am wrong about Oracle's Copyright line since it is
> > > > new file. But format of Intel's header does not match our template.
> > > >
> > > > I can suggest following to have the same form. Or we can just kepp
> > > > Intel's header as it is? What do you think?
> > > >
> > > > Thanks,
> > > > Vladimir
> > > >
> > > > /*
> > > > * Copyright (c) 2015, Intel Corporation.
> > > > * Intel Math Library (LIBM) Source Code.
> > > > * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
> > > > *
> > > > * This code is free software; you can redistribute it and/or modify it
> > > > * under the terms of the GNU General Public License version 2 only, as
> > > > * published by the Free Software Foundation.
> > > > *
> > > > * This code is distributed in the hope that it will be useful, but WITHOUT
> > > > * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> > > > * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
> > > > * version 2 for more details (a copy is included in the LICENSE file that
> > > > * accompanied this code).
> > > > *
> > > > * You should have received a copy of the GNU General Public
> > > > License version
> > > > * 2 along with this work; if not, write to the Free Software Foundation,
> > > > * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
> > > > *
> > > > * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
> > > > * or visit www.oracle.com if you need additional information or have any
> > > > * questions.
> > > > *
> > > > */
> > > >
> > > >
> > > >
> > > > On 9/3/15 12:15 PM, Viswanathan, Sandhya wrote:
> > > > > Hi Vladimir,
> > > > >
> > > > > There are other files which have two copyright notices in the OpenJDK \
> > > > > sources. Our LIBM team asked us to include Intel header as a second header \
> > > > > in this file.
> > > > > Best Regards,
> > > > > Sandhya
> > > > >
> > > > > -----Original Message-----
> > > > > From: Vladimir Kozlov [mailto:vladimir.kozlov@oracle.com]
> > > > > Sent: Thursday, September 03, 2015 11:45 AM
> > > > > To: Deshpande, Vivek R; Christian Thalinger
> > > > > Cc: Viswanathan, Sandhya
> > > > > Subject: Re: RFR (M): 8132207: Update for x86 exp in the math lib
> > > > >
> > > > > On 9/3/15 11:18 AM, Deshpande, Vivek R wrote:
> > > > > > Thanks Vladimir.
> > > > > > Also to confirm, shall I remove the Oracle Copyright ?
> > > > >
> > > > > NO! All Hotspot files have to have it. As I said add Intel's Copyright line \
> > > > > at the head of macroAssembler_x86_libm.cpp:
> > > > > > > > /*
> > > > > > > > * Copyright (c) 2015, Oracle and/or its affiliates. All rights \
> > > > > > > > reserved.
> > > > > > > > * Copyright (c) 2015, Intel Corporation. All rights reserved.
> > > > > > > > * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
> > > > >
> > > > >
> > > > > I was talking about removing next separate Copyright paragraph you have in \
> > > > > the file - you should remove that (and it duplicates information in first \
> > > > > official header anyway):
> > > > > 24 /*
> > > > > 25 * Intel Math Library (LIBM) Source Code
> > > > > 26 * Copyright (c) 2015, Intel Corporation.
> > > > > 27 *
> > > > > 28 * This program is free software; you can redistribute it and/or modify \
> > > > > it 29 * under the terms and conditions of the GNU General Public License,
> > > > > 30 * version 2, as published by the Free Software Foundation.
> > > > > 31 *
> > > > > 32 * This program is distributed in the hope it will be useful, but WITHOUT
> > > > > 33 * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> > > > > 34 * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License \
> > > > > for 35 * more details.
> > > > > 36 */
> > > > >
> > > > >
> > > > > Vladimir
> > > > >
> > > > > >
> > > > > > Regards,
> > > > > > Vivek
> > > > > >
> > > > > > -----Original Message-----
> > > > > > From: Vladimir Kozlov [mailto:vladimir.kozlov@oracle.com]
> > > > > > Sent: Thursday, September 03, 2015 11:15 AM
> > > > > > To: Deshpande, Vivek R; Christian Thalinger
> > > > > > Cc: Viswanathan, Sandhya
> > > > > > Subject: Re: RFR (M): 8132207: Update for x86 exp in the math lib
> > > > > >
> > > > > > On 9/3/15 10:58 AM, Deshpande, Vivek R wrote:
> > > > > > > Hi Vladimir
> > > > > > >
> > > > > > > Thank you for your review and comments. I will create a fresh webrev to \
> > > > > > > track on cr.openjdk and update it with your suggestions.
> > > > > > > It is 3.75x faster than current FPU code and 3x faster than the \
> > > > > > > sharedRuntime::dexp().
> > > > > >
> > > > > > Very nice!
> > > > > >
> > > > > > > Two fast_exp methods are for 64 bit and 32 bit platforms.
> > > > > >
> > > > > > I missed #ifdef in this big file.
> > > > > >
> > > > > > > We received the assembly code generated by Intel C compiler for LIBM \
> > > > > > > library, so it is hard to add comments for blocks.
> > > > > >
> > > > > > I see.
> > > > > >
> > > > > > * Intel Math Library (LIBM) Source Code
> > > > > >
> > > > > > Add Comment (not in Copyright header) that "code generated by Intel C \
> > > > > > compiler for LIBM library".
> > > > > > Thanks,
> > > > > > Vladimir
> > > > > >
> > > > > > >
> > > > > > > Please let me know if you have any suggestions.
> > > > > > >
> > > > > > > Regards,
> > > > > > > Vivek
> > > > > > > -----Original Message-----
> > > > > > > From: Vladimir Kozlov [mailto:vladimir.kozlov@oracle.com]
> > > > > > > Sent: Wednesday, September 02, 2015 5:25 PM
> > > > > > > To: Deshpande, Vivek R; Christian Thalinger
> > > > > > > Cc: Viswanathan, Sandhya
> > > > > > > Subject: Re: RFR (M): 8132207: Update for x86 exp in the math lib
> > > > > > >
> > > > > > > And I forgot main question: what is performance improvement vs current \
> > > > > > > FPU code and vs our C code SharedRuntime::dexp() implementations?
> > > > > > > Thanks,
> > > > > > > Vladimir
> > > > > > >
> > > > > > > On 9/2/15 5:16 PM, Vladimir Kozlov wrote:
> > > > > > > > Looks better. I look through all changes and they look reasonable.
> > > > > > > >
> > > > > > > > I would suggest to add few comments inside fast_exp() code to
> > > > > > > > explain what each block of code is doing.
> > > > > > > >
> > > > > > > > Why you have two MacroAssembler::fast_exp() methods?
> > > > > > > >
> > > > > > > > And copyright header should be change since it does not comply
> > > > > > > > to our format (only one line with Intel copyright should be added):
> > > > > > > >
> > > > > > > > /*
> > > > > > > > * Copyright (c) 2015, Oracle and/or its affiliates. All rights \
> > > > > > > > reserved.
> > > > > > > > * Copyright (c) 2015, Intel Corporation. All rights reserved.
> > > > > > > > * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
> > > > > > > > *
> > > > > > > > * This code is free software; you can redistribute it
> > > > > > > > and/or modify it ...
> > > > > > > >
> > > > > > > > Please, create new open webrev on cr.openjdk to track review process.
> > > > > > > >
> > > > > > > > Thanks,
> > > > > > > > Vladimir
> > > > > > > >
> > > > > > > > On 8/20/15 3:29 PM, Deshpande, Vivek R wrote:
> > > > > > > > > Hi Vladimir, Christian
> > > > > > > > >
> > > > > > > > > I have updated the patch for Math.exp() using LIBM with the
> > > > > > > > > suggestions mentioned to Sandhya during JVM language summit.
> > > > > > > > >
> > > > > > > > > Please find the patch attached with the mail.
> > > > > > > > >
> > > > > > > > > Let me know your thoughts on the changes and if you have
> > > > > > > > > further suggestions.
> > > > > > > > >
> > > > > > > > > Regards,
> > > > > > > > >
> > > > > > > > > Vivek
> > > > > > > > >
> > > > > > > > > *From:*Deshpande, Vivek R
> > > > > > > > > *Sent:* Thursday, July 23, 2015 11:01 AM
> > > > > > > > > *To:* 'hotspot-compiler-dev@openjdk.java.net'
> > > > > > > > > *Cc:* Vladimir.Kozlov@oracle.com; Viswanathan, Sandhya
> > > > > > > > > *Subject:* RFR (M): 8132207: Update for x86 exp in the math lib
> > > > > > > > >
> > > > > > > > > Hi all
> > > > > > > > >
> > > > > > > > > I would like to contribute a patch which optimizes Math.exp()
> > > > > > > > > for
> > > > > > > > > 64 and
> > > > > > > > > 32 bit X86 architecture using Intel LIBM implementation.
> > > > > > > > >
> > > > > > > > > Please review and sponsor this patch.
> > > > > > > > >
> > > > > > > > > Bug-id: https://bugs.openjdk.java.net/browse/JDK-8132207
> > > > > > > > >
> > > > > > > > > webrev:
> > > > > > > > >
> > > > > > > > > http://cr.openjdk.java.net/~mcberg/8132207/webrev.01/
> > > > > > > > >
> > > > > > > > > Thanks,
> > > > > > > > >
> > > > > > > > > Vivek
> > > > > > > > >
>
[Attachment #3 (unknown)]
<html><head><meta http-equiv="Content-Type" content="text/html \
charset=utf-8"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; \
-webkit-line-break: after-white-space;" class=""><div class="">Hi Vivek,</div><div \
class=""><br class=""></div><div class="">There are some slight build problems with \
this.</div><div class=""><br class=""></div><div class="">Probably a missling include \
of macroAssembler_x86.hpp. Try building without precompiled headers \
(USE_PRECOMPILED_HEADER=0).</div><div class=""><pre style="word-wrap: break-word; \
white-space: pre-wrap;" \
class="">/opt/jprt/T/P1/062856.iggy/s/hotspot/src/cpu/x86/vm/macroAssembler_x86_libm.cpp:192:6: \
error: incomplete type 'MacroAssembler' named in nested name specifier void \
MacroAssembler::fast_exp(XMMRegister xmm0, XMMRegister xmm1, XMMRegister xmm2, \
XMMRegister xmm3, XMMRegister xmm4, XMMRegister xmm5, XMMRegister xmm6, XMMRegister \
xmm7, Register eax, Register ecx, Register edx, Register tmp) { ^~~~~~~~~~~~~~~~
/opt/jprt/T/P1/062856.iggy/s/hotspot/src/share/vm/asm/assembler.hpp:40:7: note: \
forward declaration of 'MacroAssembler' class MacroAssembler;
^
1 error generated.
</pre></div><div class=""><br class=""></div><div class=""><br class=""></div><div \
class="">On non-intel platforms, this is an issue. supports_sse2() here should \
probably be abstracted away as "should_use_math_stub()" or something to make it more \
platform independent. Otherwise on other CPUs, supports_sse2() is simply \
undeclared.</div><div class=""><pre style="word-wrap: break-word; white-space: \
pre-wrap;" class="">/opt/jprt/T/P1/062856.iggy/s/hotspot/src/share/vm/opto/library_call.cpp \
/opt/jprt/T/P1/062856.iggy/s/hotspot/src/share/vm/opto/library_call.cpp: In member \
function 'bool LibraryCallKit::inline_math_native(vmIntrinsics::ID)':
/opt/jprt/T/P1/062856.iggy/s/hotspot/src/share/vm/opto/library_call.cpp:1765:12: \
error: 'supports_sse2' is not a member of 'VM_Version'
return VM_Version::supports_sse2() ? runtime_math(OptoRuntime::Math_D_D_Type(), \
StubRoutines::dexp(), "dexp") : ^
</pre></div><div class=""><br class=""></div><div class="">Also, I believe you tested \
this, but could you please add a unit test (somewhere to hotspot/test/compiler) that \
would verify that j.l.Math.exp() returns results as intended? Otherwise there's some \
pretty complicated code there to be proved correct just by eyeballing it.</div><div \
class=""><br class=""></div><div class="">igor</div><div class=""><br \
class=""></div><br class=""><div><blockquote type="cite" class=""><div class="">On \
Sep 16, 2015, at 2:54 PM, Deshpande, Vivek R <<a \
href="mailto:vivek.r.deshpande@intel.com" \
class="">vivek.r.deshpande@intel.com</a>> wrote:</div><br \
class="Apple-interchange-newline"><div class="">Hi Igor, <br class=""><br \
class="">Could you please sponsor this patch. Vladimir and Christian have reviewed \
the patch.<br class="">Thanks.<br class=""><br class="">Regards,<br class="">Vivek<br \
class=""><br class="">-----Original Message-----<br class="">From: Christian \
Thalinger [<a href="mailto:christian.thalinger@oracle.com" \
class="">mailto:christian.thalinger@oracle.com</a>] <br class="">Sent: Wednesday, \
September 16, 2015 2:05 PM<br class="">To: Deshpande, Vivek R<br class="">Cc: \
Vladimir Kozlov; Viswanathan, Sandhya; <a \
href="mailto:hotspot-compiler-dev@openjdk.java.net" \
class="">hotspot-compiler-dev@openjdk.java.net</a><br class="">Subject: Re: Copyright \
header question: 8132207: Update for x86 exp in the math lib<br class=""><br \
class="">Looks much better. Thanks you.<br class=""><br class=""><blockquote \
type="cite" class="">On Sep 16, 2015, at 8:10 AM, Deshpande, Vivek R <<a \
href="mailto:vivek.r.deshpande@intel.com" \
class="">vivek.r.deshpande@intel.com</a>> wrote:<br class=""><br class="">Hi \
Christian, Please let us know if the updated patch looks good to you.<br class=""><br \
class="">-----Original Message-----<br class="">From: Vladimir Kozlov [<a \
href="mailto:vladimir.kozlov@oracle.com" \
class="">mailto:vladimir.kozlov@oracle.com</a>]<br class="">Sent: Wednesday, \
September 16, 2015 10:58 AM<br class="">To: Deshpande, Vivek R; Viswanathan, Sandhya; \
<br class=""><a href="mailto:hotspot-compiler-dev@openjdk.java.net" \
class="">hotspot-compiler-dev@openjdk.java.net</a><br class="">Subject: Re: Copyright \
header question: 8132207: Update for x86 exp in <br class="">the math lib<br \
class=""><br class="">Looks good to me.<br class=""><br class="">Thanks,<br \
class="">Vladimir<br class=""><br class="">On 9/16/15 10:51 AM, Deshpande, Vivek R \
wrote:<br class=""><blockquote type="cite" class="">Hi Vladimir<br class=""><br \
class="">We have updated the patch with your suggestions and with the suggested \
copyright header.<br class=""><br class="">Bug-id:<br \
class="">https://bugs.openjdk.java.net/browse/JDK-8132207<br class="">webrev:<br \
class="">http://cr.openjdk.java.net/~mcberg/8132207/webrev.03/<br class=""><br \
class="">Thanks.<br class=""><br class="">Regards,<br class="">Vivek<br class=""><br \
class="">-----Original Message-----<br class="">From: Viswanathan, Sandhya<br \
class="">Sent: Friday, September 11, 2015 3:32 PM<br class="">To: Vladimir Kozlov; \
Deshpande, Vivek R; Christian Thalinger<br class="">Cc: david.katleman@oracle.com; \
David Holmes<br class="">Subject: RE: Copyright header question: 8132207: Update for \
x86 exp <br class="">in the math lib<br class=""><br class="">Hi Vladimir,<br \
class=""><br class="">Yes we can do that. We will send you the updated patch.<br \
class=""><br class="">Thanks,<br class="">Sandhya<br class=""><br class=""><br \
class="">-----Original Message-----<br class="">From: Vladimir Kozlov \
[mailto:vladimir.kozlov@oracle.com]<br class="">Sent: Friday, September 11, 2015 3:00 \
PM<br class="">To: Viswanathan, Sandhya; Deshpande, Vivek R; Christian Thalinger<br \
class="">Cc: david.katleman@oracle.com; David Holmes<br class="">Subject: Re: \
Copyright header question: 8132207: Update for x86 exp <br class="">in the math \
lib<br class=""><br class="">Can you do as I suggested in previous mail?<br \
class=""><br class="">/*<br class=""> * Copyright (c) 2015, Intel \
Corporation.<br class=""> * Intel Math Library (LIBM) Source Code.<br \
class=""> * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE \
HEADER.<br class=""> *<br class=""> * This code is free \
software; you can redistribute it and/or <br class="">modify it<br class=""><br \
class="">and the rest of standard Oracle template after that.<br class=""><br \
class="">Thanks,<br class="">Vladimir<br class=""><br class="">On 9/11/15 2:50 PM, \
Viswanathan, Sandhya wrote:<br class=""><blockquote type="cite" class="">Hi \
Vladimir,<br class=""><br class="">It has been a week since; please let us know how \
to proceed on the copyright header.<br class="">Vivek has everything else done and \
ready to submit the updated patch.<br class=""><br class="">Best Regards,<br \
class="">Sandhya<br class=""><br class="">-----Original Message-----<br \
class="">From: Vladimir Kozlov [mailto:vladimir.kozlov@oracle.com]<br class="">Sent: \
Thursday, September 03, 2015 2:25 PM<br class="">To: Viswanathan, Sandhya; Deshpande, \
Vivek R; Christian Thalinger<br class="">Cc: david.katleman@oracle.com; David \
Holmes<br class="">Subject: Copyright header question: 8132207: Update for x86 exp in \
<br class="">the math lib<br class=""><br class="">Hi Davis K.<br class=""><br \
class="">I hope you can help or advise us about Copyright header in new file <br \
class="">contributed by Intel:<br class=""><br \
class="">http://cr.openjdk.java.net/~mcberg/8132207/webrev.01/raw_files/new/s<br \
class="">r c /cpu/x86/vm/macroAssembler_x86_libm.cpp<br class=""><br \
class="">Currently it has 2 copyright blocks, Oracle's and Intel's.<br class="">It \
looks like I am wrong about Oracle's Copyright line since it is <br class="">new \
file. But format of Intel's header does not match our template.<br class=""><br \
class="">I can suggest following to have the same form. Or we can just kepp <br \
class="">Intel's header as it is? What do you think?<br class=""><br \
class="">Thanks,<br class="">Vladimir<br class=""><br class="">/*<br class=""> \
* Copyright (c) 2015, Intel Corporation.<br class=""> * Intel \
Math Library (LIBM) Source Code.<br class=""> * DO NOT ALTER OR REMOVE \
COPYRIGHT NOTICES OR THIS FILE HEADER.<br class=""> *<br class=""> \
* This code is free software; you can redistribute it and/or modify it<br \
class=""> * under the terms of the GNU General Public License version 2 \
only, as<br class=""> * published by the Free Software Foundation.<br \
class=""> *<br class=""> * This code is distributed in the \
hope that it will be useful, but WITHOUT<br class=""> * ANY WARRANTY; \
without even the implied warranty of MERCHANTABILITY or<br class=""> * \
FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License<br \
class=""> * version 2 for more details (a copy is included in the LICENSE \
file that<br class=""> * accompanied this code).<br class=""> \
*<br class=""> * You should have received a copy of the GNU \
General Public <br class="">License version<br class=""> * 2 along with \
this work; if not, write to the Free Software Foundation,<br class=""> * \
Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.<br class=""> \
*<br class=""> * Please contact Oracle, 500 Oracle Parkway, \
Redwood Shores, CA 94065 USA<br class=""> * or visit www.oracle.com if \
you need additional information or have any<br class=""> * questions.<br \
class=""> *<br class=""> */<br class=""><br class=""><br \
class=""><br class="">On 9/3/15 12:15 PM, Viswanathan, Sandhya wrote:<br \
class=""><blockquote type="cite" class="">Hi Vladimir,<br class=""><br class="">There \
are other files which have two copyright notices in the OpenJDK sources. Our LIBM \
team asked us to include Intel header as a second header in this file.<br \
class=""><br class="">Best Regards,<br class="">Sandhya<br class=""><br \
class="">-----Original Message-----<br class="">From: Vladimir Kozlov \
[mailto:vladimir.kozlov@oracle.com]<br class="">Sent: Thursday, September 03, 2015 \
11:45 AM<br class="">To: Deshpande, Vivek R; Christian Thalinger<br class="">Cc: \
Viswanathan, Sandhya<br class="">Subject: Re: RFR (M): 8132207: Update for x86 exp in \
the math lib<br class=""><br class="">On 9/3/15 11:18 AM, Deshpande, Vivek R \
wrote:<br class=""><blockquote type="cite" class="">Thanks Vladimir.<br class="">Also \
to confirm, shall I remove the Oracle Copyright ?<br class=""></blockquote><br \
class="">NO! All Hotspot files have to have it. As I said add Intel's Copyright line \
at the head of macroAssembler_x86_libm.cpp:<br class=""><br class=""><blockquote \
type="cite" class=""><blockquote type="cite" class=""><blockquote type="cite" \
class="">/*<br class=""> * Copyright (c) 2015, Oracle and/or its \
affiliates. All rights reserved.<br class=""> * Copyright (c) 2015, Intel \
Corporation. All rights reserved.<br class=""> * DO NOT ALTER OR REMOVE \
COPYRIGHT NOTICES OR THIS FILE HEADER.<br \
class=""></blockquote></blockquote></blockquote><br class=""><br class="">I was \
talking about removing next separate Copyright paragraph you have in the file - you \
should remove that (and it duplicates information in first official header \
anyway):<br class=""><br class=""> 24 /*<br class=""> \
25 * Intel Math Library (LIBM) Source Code<br class=""> \
26 * Copyright (c) 2015, Intel Corporation.<br class=""> \
27 *<br class=""> 28 * This program \
is free software; you can redistribute it and/or modify it<br class=""> \
29 * under the terms and conditions of the GNU General Public \
License,<br class=""> 30 * version 2, as published by the \
Free Software Foundation.<br class=""> 31 *<br class=""> \
32 * This program is distributed in the hope it will be \
useful, but WITHOUT<br class=""> 33 * ANY WARRANTY; without \
even the implied warranty of MERCHANTABILITY or<br class=""> \
34 * FITNESS FOR A PARTICULAR PURPOSE. See the GNU \
General Public License for<br class=""> 35 * more details.<br \
class=""> 36 */<br class=""><br class=""><br \
class="">Vladimir<br class=""><br class=""><blockquote type="cite" class=""><br \
class="">Regards,<br class="">Vivek<br class=""><br class="">-----Original \
Message-----<br class="">From: Vladimir Kozlov [mailto:vladimir.kozlov@oracle.com]<br \
class="">Sent: Thursday, September 03, 2015 11:15 AM<br class="">To: Deshpande, Vivek \
R; Christian Thalinger<br class="">Cc: Viswanathan, Sandhya<br class="">Subject: Re: \
RFR (M): 8132207: Update for x86 exp in the math lib<br class=""><br class="">On \
9/3/15 10:58 AM, Deshpande, Vivek R wrote:<br class=""><blockquote type="cite" \
class="">Hi Vladimir<br class=""><br class="">Thank you for your review and comments. \
I will create a fresh webrev to track on cr.openjdk and update it with your \
suggestions.<br class=""><br class="">It is 3.75x faster than current FPU code and 3x \
faster than the sharedRuntime::dexp().<br class=""></blockquote><br class="">Very \
nice!<br class=""><br class=""><blockquote type="cite" class="">Two fast_exp methods \
are for 64 bit and 32 bit platforms.<br class=""></blockquote><br class="">I missed \
#ifdef in this big file.<br class=""><br class=""><blockquote type="cite" class="">We \
received the assembly code generated by Intel C compiler for LIBM library, so it is \
hard to add comments for blocks.<br class=""></blockquote><br class="">I see.<br \
class=""><br class="">* Intel Math Library (LIBM) Source Code<br class=""><br \
class="">Add Comment (not in Copyright header) that "code generated by Intel C \
compiler for LIBM library".<br class=""><br class="">Thanks,<br class="">Vladimir<br \
class=""><br class=""><blockquote type="cite" class=""><br class="">Please let me \
know if you have any suggestions.<br class=""><br class="">Regards,<br \
class="">Vivek<br class="">-----Original Message-----<br class="">From: Vladimir \
Kozlov [mailto:vladimir.kozlov@oracle.com]<br class="">Sent: Wednesday, September 02, \
2015 5:25 PM<br class="">To: Deshpande, Vivek R; Christian Thalinger<br class="">Cc: \
Viswanathan, Sandhya<br class="">Subject: Re: RFR (M): 8132207: Update for x86 exp in \
the math lib<br class=""><br class="">And I forgot main question: what is performance \
improvement vs current FPU code and vs our C code SharedRuntime::dexp() \
implementations?<br class=""><br class="">Thanks,<br class="">Vladimir<br \
class=""><br class="">On 9/2/15 5:16 PM, Vladimir Kozlov wrote:<br \
class=""><blockquote type="cite" class="">Looks better. I look through all changes \
and they look reasonable.<br class=""><br class="">I would suggest to add few \
comments inside fast_exp() code to <br class="">explain what each block of code is \
doing.<br class=""><br class="">Why you have two MacroAssembler::fast_exp() \
methods?<br class=""><br class="">And copyright header should be change since it does \
not comply <br class="">to our format (only one line with Intel copyright should be \
added):<br class=""><br class="">/*<br class=""> * \
Copyright (c) 2015, Oracle and/or its affiliates. All rights reserved.<br class=""> \
* Copyright (c) 2015, Intel Corporation. All rights \
reserved.<br class=""> * DO NOT ALTER OR REMOVE \
COPYRIGHT NOTICES OR THIS FILE HEADER.<br class=""> \
*<br class=""> * This \
code is free software; you can redistribute it <br class="">and/or modify it ...<br \
class=""><br class="">Please, create new open webrev on cr.openjdk to track review \
process.<br class=""><br class="">Thanks,<br class="">Vladimir<br class=""><br \
class="">On 8/20/15 3:29 PM, Deshpande, Vivek R wrote:<br class=""><blockquote \
type="cite" class="">Hi Vladimir, Christian<br class=""><br class="">I have updated \
the patch for Math.exp() using LIBM with the <br class="">suggestions mentioned to \
Sandhya during JVM language summit.<br class=""><br class="">Please find the patch \
attached with the mail.<br class=""><br class="">Let me know your thoughts on the \
changes and if you have <br class="">further suggestions.<br class=""><br \
class="">Regards,<br class=""><br class="">Vivek<br class=""><br \
class="">*From:*Deshpande, Vivek R<br class="">*Sent:* Thursday, July 23, 2015 11:01 \
AM<br class="">*To:* 'hotspot-compiler-dev@openjdk.java.net'<br class="">*Cc:* \
Vladimir.Kozlov@oracle.com; Viswanathan, Sandhya<br class="">*Subject:* RFR (M): \
8132207: Update for x86 exp in the math lib<br class=""><br class="">Hi all<br \
class=""><br class="">I would like to contribute a patch which optimizes Math.exp() \
<br class="">for<br class="">64 and<br class="">32 bit X86 architecture using Intel \
LIBM implementation.<br class=""><br class="">Please review and sponsor this \
patch.<br class=""><br class="">Bug-id: \
https://bugs.openjdk.java.net/browse/JDK-8132207<br class=""><br class="">webrev:<br \
class=""><br class="">http://cr.openjdk.java.net/~mcberg/8132207/webrev.01/<br \
class=""><br class="">Thanks,<br class=""><br class="">Vivek<br class=""><br \
class=""></blockquote></blockquote></blockquote></blockquote></blockquote></blockquote></blockquote></blockquote><br \
class=""></div></blockquote></div><br class=""></body></html>
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic