<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>