investigate adding -fno-strict-aliasing to ppcle builds

[DanHeidinga]

Pretty sure we build with that option on other platforms. Can you confirm that and then update the makefiles to include the option?

We'll need to do some perf testing on JDK8 (as it has the best perf test coverage) to validate that the adding the option doesn't negatively effect startup, footprint, throughput.

Originally posted by @DanHeidinga in #2788 (comment)

[DanHeidinga]

@pdbain-ibm Since the original issue got closed when the jimage failure was fixed, I've opened this to track the investigation into no-strict-aliasing on p

[pdbain-ibm]

Thanks. I have confirmed that we use this option for Linux X86.

[pshipton]

PPC LE Builds already use the option for the bytecodeinterpreter

/usr/bin/g++ -O3 -fno-strict-aliasing -fno-rtti -fno-threadsafe-statics -fno-exceptions -g -DLINUX -D_REENTRANT -D_FILE_OFFSET_BITS=64 -fPIC -DIPv6_FUNCTION_SUPPORT -DLINUXPPC -DLINUXPPC64 -DPPC64 -m64 -Wreturn-type -Werror -Wall -Wno-non-virtual-dtor -I. -I../include -I../oti -I../util -I../gc_include -I../omr/gc/include -I../shared_common/include -I../gc_glue_java -I../nls -I../omr/include_core -DUT_DIRECT_TRACE_REGISTRATION -DTR_HOST_POWER -c BytecodeInterpreter.cpp

https://github.com/eclipse/openj9/blob/a640b21101e4b64877dc6db87220a2141532868d/runtime/makelib/targets.mk.linux.inc.ftl#L150

[pshipton]

Not sure why the option is only used for the bytecodeinterpreter, but it should be used everywhere.

[pdbain-ibm]

Not used for jnicsup.cpp:

/usr/bin/g++ -save-temps -DOPENJ9_BUILD -O3 -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=1 -fno-exceptions -fno-rtti -fno-threadsafe-statics -g -DLINUX -D_REENTRANT -D_FILE_OFFSET_BITS=64 -fpic -DIPv6_FUNCTION_SUPPORT -DLINUXPPC -m64 -DLINUXPPC64 -DPPC64 -Wreturn-type -Werror -I. -I../include -I../oti -I../util -I../gc_include -I../omr/gc/include -I../shared_common/include -I../gc_glue_java -I../nls -I../omr/include_core -DUT_DIRECT_TRACE_REGISTRATION -DTR_HOST_POWER -c jnicsup.cpp

[pdbain-ibm]

I chatted with @zl-wang about this. Adding -fno-strict-aliasing will probably degrade performance due to more conservative code generation. I suggest we investigate removing this option where possible.

[DanHeidinga]

I would strongly prefer to see benchmark results with and without. Compilers are relying more and more on strict aliasing for performance and our current code doesn't obey the strict aliasing requirements.

I would rather add the option and avoid the long bug tail than try to find every place we've violated strict aliasing.

[pdbain-ibm]

@DanHeidinga I will see about benchmarking the VM with -fno-strict-aliasing.

Given that jnicsup.cpp currently uses strict aliasing and we haven't seen any bugs since I fixed the aliasing errors, are you suggesting disabling strict aliasing in this file as prophylaxis?

@gacholio your thoughts?

[gacholio]

I have no idea what the option does.

[DanHeidinga]

are you suggesting disabling strict aliasing in this file as prophylaxis?

My preference is to enable -fno-strict-aliasing globally to match other platforms and to pinch off the potential bug tail.

[zl-wang]

globally as applying to all OMR and OpenJ9 files?
I am not sure if it is a good idea. Certainly, that option alone will not provide extra assurance of correctness (since only some relevant optimizations are disabled). In any case, let's wait and see the performance comparison with/without.

[DanHeidinga]

globally as applying to all OMR and OpenJ9 files?

We should match what's been done on other platforms (notable x) as the code base has not been written with strict aliasing in mind. I was surprised to find we didn't already have it option set.

[pdbain-ibm]

Sent test builds for performance testing. The test machines have an earlier version of glibc:

JVMJ9VM011W Unable to load j9jit29: /lib64/ld64.so.2: version `GLIBC_2.22' not found (required by /java/perffarm/sdks/pxl6480sr6-pplce_strict/sdk/jre/lib/ppc64/compressedrefs/libj9jit29.so)

I am rebuilding with what I hope is the correct version of glibc.so.

[mamatha-jv]

Please find performance testing results below(no regressions seen):-

LibertyStartupDT-9dev-4way-0-256-qs_Footprint in kb
99.77%

LibertyStartupDT-9dev-4way-0-256-qs_Startup time in ms
98.59%

LibertyStartup-9dev-4way-0-256-qs_Footprint in kb
99.92%

LibertyStartup-9dev-4way-0-256-qs_Startup time in ms
99.42%

LibertyDayTrader3-9dev-4way-LargeThreadPool_Adjusted Single Server Memory
100.18%

LibertyDayTrader3-9dev-4way-LargeThreadPool_Throughput
99.61%

LibertyDayTrader3-9dev-4way-LargeThreadPoolwarm_Adjusted Single Server Memory
99.86%

LibertyDayTrader3-9dev-4way-LargeThreadPoolwarm_Throughput
99.26%

[gacholio]

Sounds like this should probably be enabled on all gcc compiles.

[pdbain-ibm]

Pull request for PPCLE: #3087
Leaving this open to check for other places to enable this option.

[pdbain-ibm]

Working on OMR.

• MacOS and zTPF already enable no-strict-aliasing in omr/omrmakefiles/rules.osx.mk and omr/omrmakefiles/rules.ztpf.mk
• omr/omrmakefiles/rules.linux.mk enables this option for zLinux
• It is not currently enabled on xLinux, aarch64, ARM, or LinuxPPC; Adding that.

[pdbain-ibm]

Ran a build with preliminary pull request eclipse-omr/omr#3185.
Requested performance testing on Linux X86 and Linux PPC LE.

[pdbain-ibm]

eclipse-omr/omr#3185 is merged. OMR and OpenJ9 now have -fno-strict-aliasing enabled on all GCC platforms. Can this issue be closed?
Thanks

[gacholio]

JIT?

[pdbain-ibm]

Don't the OMR changes cover the JIT?

[charliegracie]

No the JIT does not use the OMR makefiles similar to how they do not use the OpenJ9 makefiles.

[pdbain-ibm]

Pull request for jitbuilder changes opened: eclipse-omr/omr#3360. Tested in personal build on all platforms.

[pdbain-ibm]

The JIT makefiles have been updated. Please close this issue unless there are other components which require changes.
Thank you.

[DanHeidinga]

Thanks @pdbain-ibm