[PING] [8u] RFR: 8210425: [x86] sharedRuntimeTrig/sharedRuntimeTrans compiled without optimization

[Andrew John Hughes]

On 09/05/2019 13:39, sgehwolf at 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
> 

Looks good to me.

Thanks,
-- 
Andrew :)

Senior Free Java Software Engineer
Red Hat, Inc. (http://www.redhat.com)

PGP Key: ed25519/0xCFDA0F9B35964222 (hkp://keys.gnupg.net)
Fingerprint = 5132 579D D154 0ED2 3E04  C5A0 CFDA 0F9B 3596 4222
https://keybase.io/gnu_andrew