Looks good.
Paul
On 5/9/19, 5:39 AM, "hotspot-dev on behalf of sgehw...@redhat.com"
<hotspot-dev-boun...@openjdk.java.net on behalf of sgehw...@redhat.com> wrote:
Ping?
On Fri, 2019-05-03 at 18:35 +0200, Severin Gehwolf wrote:
> Hi,
>
> Could I please get reviews for this 8u backport of an optimization fix
> pertaining to the hotspot copy of fdlibm? Historically,
> sharedRuntimeTrans.cpp and sharedRuntimeTrig.cpp have been compiled
> with optimization turned off on x86/x86_64. As JDK-8210416 will be
> backported to JDK 8u fixing optimization for fdlibm itself, so should
> be the hotspot copy in OpenJDK 8u. The build system for hotspot in JDK
> 8u is very different, so the original fix for JDK 12 doesn't apply.
> It's a complete rewrite. Please see this webrev which shows the
> prerequisite patch. It adds the FDLIBM_CFLAGS logic:
> http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8210416/jdk8/02/
>
> It also fixes a bug on ppc64{,le} where optimization is being turned on
> in hotspot (-O3), but not providing the -ffp-contract=off flag (or
> older equivalent).
>
> Before (on ppc64le):
> /bin/g++ -DLINUX -D_GNU_SOURCE -DPPC64 -DPRODUCT -I.
-I/home/openjdk/jdk8u/hotspot/src/share/vm/prims
-I/home/openjdk/jdk8u/hotspot/src/share/vm
-I/home/openjdk/jdk8u/hotspot/src/share/vm/precompiled
-I/home/openjdk/jdk8u/hotspot/src/cpu/ppc/vm
-I/home/openjdk/jdk8u/hotspot/src/os_cpu/linux_ppc/vm
-I/home/openjdk/jdk8u/hotspot/src/os/linux/vm
-I/home/openjdk/jdk8u/hotspot/src/os/posix/vm -I../generated
-DHOTSPOT_RELEASE_VERSION="\"25.212-b04\"" -DHOTSPOT_BUILD_TARGET="\"product\""
-DHOTSPOT_BUILD_USER="\"openjdk\"" -DHOTSPOT_LIB_ARCH=\"ppc64le\"
-DHOTSPOT_VM_DISTRO="\"OpenJDK\"" -DTARGET_OS_FAMILY_linux -DTARGET_ARCH_ppc
-DTARGET_ARCH_MODEL_ppc_64 -DTARGET_OS_ARCH_linux_ppc
-DTARGET_OS_ARCH_MODEL_linux_ppc_64 -DTARGET_COMPILER_gcc -DCOMPILER2 -fPIC
-fno-rtti -fno-exceptions -D_REENTRANT -fcheck-new -fvisibility=hidden -m64
-pipe -fno-strict-aliasing -fno-omit-frame-pointer -O3 -g -D_LP64=1
-DVM_LITTLE_ENDIAN -DABI_ELFv2 -mcpu=power7 -mtune=power8
-minsert-sched-nops=regroup_exact -mno-multiple -mno-string -Werror
-Wpointer-arith -Wsign-compare -Wundef -Wunused-function -Wunused-value
-std=gnu++98 -c -MMD -MP -MF ../generated/dependencies/sharedRuntimeTrig.o.d
-fpch-deps -o sharedRuntimeTrig.o
/home/openjdk/jdk8u/hotspot/src/share/vm/runtime/sharedRuntimeTrig.cpp
> After (on ppc64le; Note -ffp-contract=off flag):
> /bin/g++ -DLINUX -D_GNU_SOURCE -DPPC64 -DPRODUCT -I.
-I/home/openjdk/jdk8u/hotspot/src/share/vm/prims
-I/home/openjdk/jdk8u/hotspot/src/share/vm
-I/home/openjdk/jdk8u/hotspot/src/share/vm/precompiled
-I/home/openjdk/jdk8u/hotspot/src/cpu/ppc/vm
-I/home/openjdk/jdk8u/hotspot/src/os_cpu/linux_ppc/vm
-I/home/openjdk/jdk8u/hotspot/src/os/linux/vm
-I/home/openjdk/jdk8u/hotspot/src/os/posix/vm -I../generated
-DHOTSPOT_RELEASE_VERSION="\"25.212-b04\"" -DHOTSPOT_BUILD_TARGET="\"product\""
-DHOTSPOT_BUILD_USER="\"openjdk\"" -DHOTSPOT_LIB_ARCH=\"ppc64le\"
-DHOTSPOT_VM_DISTRO="\"OpenJDK\"" -DTARGET_OS_FAMILY_linux -DTARGET_ARCH_ppc
-DTARGET_ARCH_MODEL_ppc_64 -DTARGET_OS_ARCH_linux_ppc
-DTARGET_OS_ARCH_MODEL_linux_ppc_64 -DTARGET_COMPILER_gcc -DCOMPILER2 -fPIC
-fno-rtti -fno-exceptions -D_REENTRANT -fcheck-new -fvisibility=hidden -m64
-pipe -fno-strict-aliasing -fno-omit-frame-pointer -O3 -ffp-contract=off
-D_LP64=1 -DVM_LITTLE_ENDIAN -DABI_ELFv2 -mcpu=power7 -mtune=power8
-minsert-sched-nops=regroup_exact -mno-multiple -mno-string -Werror
-Wpointer-arith -Wsign-compare -Wundef -Wunused-function -Wunused-value
-std=gnu++98 -c -MMD -MP -MF ../generated/dependencies/sharedRuntimeTrig.o.d
-fpch-deps -o sharedRuntimeTrig.o
/home/openjdk/jdk8u/hotspot/src/share/vm/runtime/sharedRuntimeTrig.cpp
>
> Bug: https://bugs.openjdk.java.net/browse/JDK-8210425
> webrev:
http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8210425/jdk8/02/webrev
>
> Testing: Builds on x86_64 (server/Zero). Manual inspection of compile
> log of sharedRuntimeTrans.cpp/sharedRuntimeTrig.cpp. Same for ppc64le.
> Note that we've been using this patch downstream for a while now. We
> haven't seen any issues so far.
>
> Thanks,
> Severin
