On Tue, 5 Mar 2024 09:10:50 GMT, Joachim Kern <jk...@openjdk.org> wrote:
>>> > What does this mean? That you are not using xlc at all? Or is it clang >>> > but still with an xlc frontend, so all xlc flags etc need to stay? >>> >>> The `xlc` toolchain is for the compiler versions up to 16 (xlclang++); the >>> `clang` toolchain is for the compiler versions 17 (ibm-clang++_r) and >>> higher. For the 17 Compiler the frontend is `clang`-ish and we are using >>> the `clang` flags instead of the `xlc` flags. `toolchain.m4` decides on the >>> basis of the found compiler which toolchain to use as default. >>> >>> ``` >>> # On AIX the default toolchain depends on the installed (found) compiler >>> # xlclang++ -> xlc toolchain >>> # ibm-clang++_r -> clang toolchain >>> # The compiler is searched on the PATH and TOOLCHAIN_PATH >>> # xlclang++ has precedence over ibm-clang++_r if both are installed >>> ``` >>> >>> So, if we set the minimum compiler level for AIX to 17, we can remove the >>> xlc toolchain at all. We cannot remove every reference to xlc, because at >>> least some headers we still use the xlc version (globalDefinitions_xlc.hpp) >> >> This suggests there might be more that needs to be done here than simply >> updating TOOLCHAIN_MINIMUM_VERSION_xlc. I spent some time looking at the >> relevant code, but keep getting lost in the distinction between xlc and >> clang. >> Does updating that variable as proposed even work at all? >> >> I'm going to need some help from you aix-ppc maintainer folks. > >> > > What does this mean? That you are not using xlc at all? Or is it clang >> > > but still with an xlc frontend, so all xlc flags etc need to stay? >> > >> > >> > The `xlc` toolchain is for the compiler versions up to 16 (xlclang++); the >> > `clang` toolchain is for the compiler versions 17 (ibm-clang++_r) and >> > higher. For the 17 Compiler the frontend is `clang`-ish and we are using >> > the `clang` flags instead of the `xlc` flags. `toolchain.m4` decides on >> > the basis of the found compiler which toolchain to use as default. >> > ``` >> > # On AIX the default toolchain depends on the installed (found) compiler >> > # xlclang++ -> xlc toolchain >> > # ibm-clang++_r -> clang toolchain >> > # The compiler is searched on the PATH and TOOLCHAIN_PATH >> > # xlclang++ has precedence over ibm-clang++_r if both are installed >> > ``` >> > >> > >> > >> > >> > >> > >> > >> > >> > >> > >> > >> > So, if we set the minimum compiler level for AIX to 17, we can remove the >> > xlc toolchain at all. We cannot remove every reference to xlc, because at >> > least some headers we still use the xlc version (globalDefinitions_xlc.hpp) >> >> This suggests there might be more that needs to be done here than simply >> updating TOOLCHAIN_MINIMUM_VERSION_xlc. I spent some time looking at the >> relevant code, but keep getting lost in the distinction between xlc and >> clang. Does updating that variable as proposed even work at all? >> >> I'm going to need some help from you aix-ppc maintainer folks. > > As I already mentioned, This PR is just the start shot to remove the support > for the old xlc compilers below version 17. This means removing the xlc > toolchain support in a follow up PR by our team. This is feasible, because > the open xl compilers starting with version 17 are using the clang toolchain. > So, if this PR is through we feel empowered to remove the xlc toolchain. > Your current version still serves the purpose of disallowing usage of xlc 16 > and below. Users of the old compiler will get to know which compiler to use > at minimum. Users of any Open XL compilers are not affected because > "TOOLCHAIN_MINIMUM_VERSION_xlc" is not used for that AFAICS. But that's ok > IMHO because the minimum clang check will be in place and the supported > compilers are documented > (https://wiki.openjdk.org/display/Build/Supported+Build+Platforms). So, I > think this PR could get integrated and the old build pipeline get removed in > a separate PR. Do you agree @JoKern65? The non-use of `TOOLCHAIN_MINIMUM_VERSION_xlc` when using the version 17 compilers is one of the things that worried me. (So I did correctly figure out it wouldn't be used.) I think this means one could inadvertently use 17.1.0 rather than the desired requirement of 17.1.1. I think raising the minimum clang version to 13 (as proposed in https://github.com/openjdk/jdk/pull/17862) doesn't catch that either, since 17.1.0 is based on clang 13. But if you folks are okay with this as an interim step, then I will go ahead. ------------- PR Comment: https://git.openjdk.org/jdk/pull/17857#issuecomment-1978476778
