Hello Matthias,
On 2019-05-08 06:27, Baesken, Matthias wrote:
Hello, I looked a bit more into it .
It seems to me , that when -ffp-contract=off is available which is the
case with current gcc versions , we want to optimize the 2 special files (
sharedRuntimeTrig.cpp / sharedRuntimeTrans.cpp ).
see the following comments :
jdk/make/hotspot/lib/JvmOverrideFiles.gmk
47# If the FDLIBM_CFLAGS variable is non-empty we know
48# that the fdlibm-fork in hotspot can get optimized
49# by using -ffp-contract=off on GCC/Clang platforms.
......
58 BUILD_LIBJVM_sharedRuntimeTrig.cpp_CXXFLAGS := -DNO_PCH $(FDLIBM_CFLAGS)
$(LIBJVM_FDLIBM_COPY_OPT_FLAG)
59 BUILD_LIBJVM_sharedRuntimeTrans.cpp_CXXFLAGS := -DNO_PCH $(FDLIBM_CFLAGS)
$(LIBJVM_FDLIBM_COPY_OPT_FLAG)
60
Will this not just resolve itself if you also add -D_FORTIFY_SOURCE=0 to
C_O_FLAG_NONE (and all the other optimization flag variables that have
the value "-O0")?
But still, setting both -O3 and -O2 in one compile call looks not nice to me .
This may not look nice, but is how we have to do things. The last flag
on a compiler command line takes precedence. We rely on this to override
general flags with more specific ones (typically a general flag for the
whole library with specific ones for certain compilation units). This
technique is quite common.
In case of ancient gcc ***without*** -ffp-contract=off , we might still
run into issues for these 2 special files when _FORTIFY_SOURCE is set .
Don't know if this is still relevant .
In case we want to be on the very safe side , we might need to filter out
-D_FORTIFY_SOURCE=2 for these 2 compilation units .
We don't want to filter out flags. It creates very brittle code that is
likely to break in the future.
For the patch, I think it would make sense to introduce a variable for
the value -D_FORTIFY_SOURCE=2 (and
-D_FORTIFY_SOURCE=0/-U_FORTIFY_SOURCE) to avoid repeating it.
/Erik
Best regards, Matthias
Hi David, thanks for the comment .
Currently I do not see the issue in our fastdebug builds .
So I think the opt-flag filtering got changed/removed in the years after the
issues were reported .
https://bugs.openjdk.java.net/browse/JDK-8047952
mentions special O-level settings for sharedRuntimeTrig.cpp and
sharedRuntimeTrans.cpp .
But the files have optimization set in both fastdebug and opt builds :
Linux x86_64 gcc-7 based builds :
fastdebug build (with the added -D_FORTIFY_SOURCE=2 flag) :
-Werror -O3 -D_FORTIFY_SOURCE=2 -DNO_PCH -ffp-contract=off -O2 -
D_FORTIFY_SOURCE=2
Opt build (without -D_FORTIFY_SOURCE=... ) :
-O3 -DNO_PCH -ffp-contract=off -O2 ....
(btw. the setting of both -O3 AND -O2 looks strange to me , but that’s
unrelated to my change ; I noticed that already in OpenJDK 11 ).
Best regards, Matthias
Hi Matthias,
On 8/05/2019 6:05 pm, Baesken, Matthias wrote:
Hello, here is a webrev, I used the existing bug
"JDK-8130017 : use _FORTIFY_SOURCE in gcc fastdebug builds"
Hope that’s fine .
That is fine, but please add a comment to the bug explaining exactly how
you fixed the issue and how the issues raised in the bug description
regarding optimisation levels have been addressed.
Not a review - I'll leave that to build team. The proof of this will be
in the building and testing.
Thanks,
David
https://bugs.openjdk.java.net/browse/JDK-8130017
http://cr.openjdk.java.net/~mbaesken/webrevs/8130017.0/
Our internal OpenJDK Linux (x86_64, ppc64, ppc64le , s390x) fastdebug
builds are fine with the added flag .
