Re: RFR: JDK-8160926: FLAGS_COMPILER_CHECK_ARGUMENTS doesn't handle cross-compilation
On 2019-02-05 18:55, Erik Joelsson wrote: On 2019-02-05 01:12, Magnus Ihse Bursie wrote: On 2019-02-05 01:36, Erik Joelsson wrote: Please review this fix for configure flags checking. The macros for checking compiler capabilities were not able to handle cross compilation very well. With this fix, they accept an optional PREFIX argument, instructing them to check the compiler with the given prefix (which is either empty or BUILD_). Using this, I have moved all calls to the macros into BUILD/TARGET specific macro bodies so that we correctly check both the build and target compilers for valid flags. I have verified by running cross compilation configurations of linux-aarch64 using both the same and different versions of GCC for build and target, and compared the output in both configure.log and the spec files. I've also run tier1 (equivalent to submission forest) and a COMPARE_BUILD run on Oracle's main platforms as well. The change unfortunately required adding another prefix type format since our compiler/tool variables use /BUILD_ as prefix and the existing formats were TARGET_/BUILD_ and /OPENJDK_TARGET_. I think we could benefit greatly from unifying these. I've also touched some white space nearby my changes as well as fixed a trivial error message about "linker" not being found by quoting the LINKER_NAME assignment in toolchain.m4. Bug: https://bugs.openjdk.java.net/browse/JDK-8160926 Webrev: http://cr.openjdk.java.net/~erikj/8160926/webrev.01 Looks good. Thanks for finally getting this fixed. Thanks! A related problem, that perhaps should be fixed at the same time, is that we need to add -Werror to provoke an actual warning message that fails. This has been done manually in some (but not all) cases where we call FLAGS_COMPILER_CHECK_ARGUMENTS. It might depend on version of gcc, or what kind of options we are testing -- I don't remember the detail. But I think it would be safe, and prudent, to let FLAGS_COMPILER_CHECK_ARGUMENTS automatically add $CFLAGS_WARNINGS_ARE_ERRORS to the flag tested. (Assuming that the definition of CFLAGS_WARNINGS_ARE_ERRORS is available at the time of first call to FLAGS_COMPILER_CHECK_ARGUMENTS, but I think it is, or rather, should be.) I agree and I did react to this, but I don't really have the time to spend right now so will push this as is for now. Fair enough. Do you know if there's an open JBS issue on the problem? And if not, could you please file one? /Magnus /Erik /Magnus /Erik
Re: RFR: JDK-8160926: FLAGS_COMPILER_CHECK_ARGUMENTS doesn't handle cross-compilation
On 2019-02-05 01:12, Magnus Ihse Bursie wrote: On 2019-02-05 01:36, Erik Joelsson wrote: Please review this fix for configure flags checking. The macros for checking compiler capabilities were not able to handle cross compilation very well. With this fix, they accept an optional PREFIX argument, instructing them to check the compiler with the given prefix (which is either empty or BUILD_). Using this, I have moved all calls to the macros into BUILD/TARGET specific macro bodies so that we correctly check both the build and target compilers for valid flags. I have verified by running cross compilation configurations of linux-aarch64 using both the same and different versions of GCC for build and target, and compared the output in both configure.log and the spec files. I've also run tier1 (equivalent to submission forest) and a COMPARE_BUILD run on Oracle's main platforms as well. The change unfortunately required adding another prefix type format since our compiler/tool variables use /BUILD_ as prefix and the existing formats were TARGET_/BUILD_ and /OPENJDK_TARGET_. I think we could benefit greatly from unifying these. I've also touched some white space nearby my changes as well as fixed a trivial error message about "linker" not being found by quoting the LINKER_NAME assignment in toolchain.m4. Bug: https://bugs.openjdk.java.net/browse/JDK-8160926 Webrev: http://cr.openjdk.java.net/~erikj/8160926/webrev.01 Looks good. Thanks for finally getting this fixed. Thanks! A related problem, that perhaps should be fixed at the same time, is that we need to add -Werror to provoke an actual warning message that fails. This has been done manually in some (but not all) cases where we call FLAGS_COMPILER_CHECK_ARGUMENTS. It might depend on version of gcc, or what kind of options we are testing -- I don't remember the detail. But I think it would be safe, and prudent, to let FLAGS_COMPILER_CHECK_ARGUMENTS automatically add $CFLAGS_WARNINGS_ARE_ERRORS to the flag tested. (Assuming that the definition of CFLAGS_WARNINGS_ARE_ERRORS is available at the time of first call to FLAGS_COMPILER_CHECK_ARGUMENTS, but I think it is, or rather, should be.) I agree and I did react to this, but I don't really have the time to spend right now so will push this as is for now. /Erik /Magnus /Erik
Re: RFR: JDK-8160926: FLAGS_COMPILER_CHECK_ARGUMENTS doesn't handle cross-compilation
On 2019-02-05 01:36, Erik Joelsson wrote: Please review this fix for configure flags checking. The macros for checking compiler capabilities were not able to handle cross compilation very well. With this fix, they accept an optional PREFIX argument, instructing them to check the compiler with the given prefix (which is either empty or BUILD_). Using this, I have moved all calls to the macros into BUILD/TARGET specific macro bodies so that we correctly check both the build and target compilers for valid flags. I have verified by running cross compilation configurations of linux-aarch64 using both the same and different versions of GCC for build and target, and compared the output in both configure.log and the spec files. I've also run tier1 (equivalent to submission forest) and a COMPARE_BUILD run on Oracle's main platforms as well. The change unfortunately required adding another prefix type format since our compiler/tool variables use /BUILD_ as prefix and the existing formats were TARGET_/BUILD_ and /OPENJDK_TARGET_. I think we could benefit greatly from unifying these. I've also touched some white space nearby my changes as well as fixed a trivial error message about "linker" not being found by quoting the LINKER_NAME assignment in toolchain.m4. Bug: https://bugs.openjdk.java.net/browse/JDK-8160926 Webrev: http://cr.openjdk.java.net/~erikj/8160926/webrev.01 Looks good. Thanks for finally getting this fixed. A related problem, that perhaps should be fixed at the same time, is that we need to add -Werror to provoke an actual warning message that fails. This has been done manually in some (but not all) cases where we call FLAGS_COMPILER_CHECK_ARGUMENTS. It might depend on version of gcc, or what kind of options we are testing -- I don't remember the detail. But I think it would be safe, and prudent, to let FLAGS_COMPILER_CHECK_ARGUMENTS automatically add $CFLAGS_WARNINGS_ARE_ERRORS to the flag tested. (Assuming that the definition of CFLAGS_WARNINGS_ARE_ERRORS is available at the time of first call to FLAGS_COMPILER_CHECK_ARGUMENTS, but I think it is, or rather, should be.) /Magnus /Erik
RFR: JDK-8160926: FLAGS_COMPILER_CHECK_ARGUMENTS doesn't handle cross-compilation
Please review this fix for configure flags checking. The macros for checking compiler capabilities were not able to handle cross compilation very well. With this fix, they accept an optional PREFIX argument, instructing them to check the compiler with the given prefix (which is either empty or BUILD_). Using this, I have moved all calls to the macros into BUILD/TARGET specific macro bodies so that we correctly check both the build and target compilers for valid flags. I have verified by running cross compilation configurations of linux-aarch64 using both the same and different versions of GCC for build and target, and compared the output in both configure.log and the spec files. I've also run tier1 (equivalent to submission forest) and a COMPARE_BUILD run on Oracle's main platforms as well. The change unfortunately required adding another prefix type format since our compiler/tool variables use /BUILD_ as prefix and the existing formats were TARGET_/BUILD_ and /OPENJDK_TARGET_. I think we could benefit greatly from unifying these. I've also touched some white space nearby my changes as well as fixed a trivial error message about "linker" not being found by quoting the LINKER_NAME assignment in toolchain.m4. Bug: https://bugs.openjdk.java.net/browse/JDK-8160926 Webrev: http://cr.openjdk.java.net/~erikj/8160926/webrev.01 /Erik
