[jdk8u-dev] RFR: 8342822: jdk8u432-b06 does not compile on AIX [v2]

Mon Dec 16 22:51:41 UTC 2024

On Thu, 7 Nov 2024 11:40:27 GMT, Varada M <varadam at openjdk.org> wrote:

>> Use of llabs() for jlong resolves the build error on AIX.
>> Performed jtreg testing for hotspot/test on both aix-ppc64 and linux-ppc64le and no related failures observed 
>> 
>> JBS : [JDK-8342822](https://bugs.openjdk.org/browse/JDK-8342822)
>
> Varada M has updated the pull request incrementally with one additional commit since the last revision:
> 
>   8342822: jdk8u432-b06 does not compile on AIX

Hi, after getting access to an _AIX system_ (thanks @sxa) I have a clearer understanding of what is going on.

# Tests I executed in _AIX 7.2_

All the tests have the following structure, I will specify the C++ snippet and the results.

cat >test.cpp <<'EOF'
// C++ snippet goes here
EOF
/opt/IBM/xlC/13.1.3/bin/xlC_r -q64 -c -o/dev/null test.cpp
/opt/IBM/xlC/16.1.0/bin/xlC_r -q64 -c -o/dev/null test.cpp
rm -f test.cpp

## Finding the combination that causes the failure

### Snippets that worked

All the following snippets worked, both with `xlC` 13.1.3 and 16.1.0.

#### Use `stdlib.h` alone, with `abs()`

#include <stdlib.h>

long test() {
    return abs((long)1);
}

#### Use `cstdlib` alone, with `std::abs()`

#include <cstdlib>

long test() {
    return std::abs((long)1);
}

#### Use `cstdlib` with `std::abs()`, but also include `cmath` before `cstdlib`

#include <cmath>
#include <cstdlib>

long test() {
    return std::abs((long)1);
}

#### Use `cstdlib` with `std::abs()`, but also include `cmath` after `cstdlib`

#include <cstdlib>
#include <cmath>

long test() {
    return std::abs((long)1);
}

### Snippets that failed

The following snippets failed in the same way, both with `xlC` 13.1.3 and 16.1.0. See the error at the end.

#### Use `stdlib.h` with `abs()`, but also include `cmath` before `cstdlib`

#include <cmath>
#include <stdlib.h>

long test() {
    return abs((long)1);
}

#### Use `stdlib.h` with `abs()`, but also include `cmath` after `cstdlib`

#include <stdlib.h>
#include <cmath>

long test() {
    return abs((long)1);
}

#### Error

"test.cpp", line 5.12: 1540-0219 (S) The call to "abs" has no best match.
"test.cpp", line 5.16: 1540-1229 (I) Argument number 1 is an rvalue of type "long".
"/usr/include/math.h", line 1033.20: 1540-1202 (I) No candidate is better than "abs(long double)".
"test.cpp", line 5.16: 1540-1231 (I) The conversion from argument number 1 to "long double" uses "a floating point-integral conversion".
"/usr/include/math.h", line 1003.14: 1540-1202 (I) No candidate is better than "abs(float)".
"test.cpp", line 5.16: 1540-1231 (I) The conversion from argument number 1 to "float" uses "a floating point-integral conversion".
"/usr/include/math.h", line 1001.15: 1540-1202 (I) No candidate is better than "abs(double)".
"test.cpp", line 5.16: 1540-1231 (I) The conversion from argument number 1 to "double" uses "a floating point-integral conversion".
"/usr/include/stdlib.h", line 829.18: 1540-1202 (I) No candidate is better than "abs(int)".
"test.cpp", line 5.16: 1540-1231 (I) The conversion from argument number 1 to "int" uses "an integral conversion".

### Snippets that resolve the issue

It seems the problem is the conjunction of `cmath` with `stdlib.h`, this looks pretty similar to [IY68511: NO ABS(LONG) IMPLEMENTATION IN <CSTDLIB\>](https://www.ibm.com/support/pages/apar/IY68511):
> ## Error description
>
> There is currently no implementation for `abs(long)` functions in `<cstdlib>`, but `abs(long)` is defined in the C++ Standard.
>
> ## Local fix
>
> Use `<cmath>` without `<cstdlib>`
>
> ## Problem summary
>
> `abs(long)` was not defined in the library implementation.
>
> ## Problem conclusion
>
> In order to use `abs(long)`, the macro `__ABS_LONG__` must be defined if using `stdlib.h` instead of `cstdlib`, or if `stdlib.h` occurs before `cstdlib` in the include path.

The following snippets make it work by adding a third element (inspiration from *Problem conclusion*).

#### Define the `__ABS_LONG__` macro as suggested

#define __ABS_LONG__
#include <stdlib.h>
#include <cmath>

long test() {
    return abs((long)1);
}

NOTE: `__ABS_LONG__` must be defined at the beginning, any combination defining `__ABS_LONG__` after some of the includes, fails.

#### Include `cstdlib` in addition to `stdlib.h`

#include <cstdlib>
#include <stdlib.h>
#include <cmath>

long test() {
    return abs((long)1);
}

NOTES:

* This succeeds for all the 6 possible ways of arranging the 3 includes
* If I replace `cstdlib` with `iostream` I get the same results (for the 6 possible arrangements)
    * I don't see a direct `#include <cstdlib>` from `iostream`, but I assume an indirect inclusion or this somehow implying the definition of `__ABS_LONG__`
    * This originally made it more difficult to reproduce the issue, as I was basing on my previous snippets, which were including `iostream` to use `std::cout`

# Conclusions

For 11u, I would discard the compiler version as a possible reason why we aren't experiencing this issue (given all the previous tests were identical in both versions). I would lean towards a different preprocessor result, due to a different topology of the includes graph.

For 8u, in my opinion, the acceptable fixes would include:

* Passing `-D__ABS_LONG__` to `xlC`
* Including `cstdlib` in `hotspot/src/share/vm/opto/superword.hpp`, which is the one using `abs(long)`
    * This shouldn't break other platforms builds, but we would need to test them

I've tested both by re-executing the failed build command (not the whole build) and seem to be working. Here is the build command I've obtained building with `make_64 LOG=debug CONF=aix-ppc64-normal-server-release images`, in case anyone wants to give a look to the compiler flags:

# PWD: /home/fferrari/jdk8u/build/aix-ppc64-normal-server-release/hotspot/aix_ppc64_compiler2/product
rm -f loopnode.o
/opt/IBM/xlC/13.1.3/bin/xlC_r -DAIX -DPPC64 -DPRODUCT \
  -I/home/fferrari/jdk8u/hotspot/src/share/vm/prims \
  -I/home/fferrari/jdk8u/hotspot/src/share/vm \
  -I/home/fferrari/jdk8u/hotspot/src/share/vm/precompiled \
  -I/home/fferrari/jdk8u/hotspot/src/cpu/ppc/vm \
  -I/home/fferrari/jdk8u/hotspot/src/os_cpu/aix_ppc/vm \
  -I/home/fferrari/jdk8u/hotspot/src/os/aix/vm \
  -I/home/fferrari/jdk8u/hotspot/src/os/posix/vm \
  -I../generated \
  -DHOTSPOT_RELEASE_VERSION=""25.442-b00"" \
  -DHOTSPOT_BUILD_TARGET=""product"" -DHOTSPOT_BUILD_USER=""fferrari"" \
  -DHOTSPOT_LIB_ARCH="ppc64" -DHOTSPOT_VM_DISTRO=""OpenJDK"" \
  -DTARGET_OS_FAMILY_aix -DTARGET_ARCH_ppc -DTARGET_ARCH_MODEL_ppc_64 \
  -DTARGET_OS_ARCH_aix_ppc -DTARGET_OS_ARCH_MODEL_aix_ppc_64 \
  -DTARGET_COMPILER_xlc -DINCLUDE_JFR=0 -DCOMPILER2 -qpic=large -qnortti \
  -qnoeh -D_REENTRANT -D__STDC_FORMAT_MACROS -q64 -O3 -qalias=noansi -g \
  -qarch=ppc64 -qtune=balanced -qinlglue -qstrict -qxflag=ASMMIDCOALFIX \
  -qxflag=asmfastsync -qhot=level=1 -qignerrno -qinline -q64 -qlanglvl=c99vla \
  -qlanglvl=noredefmac -qsuppress=1540-0198 -qsuppress=1540-1090 \
  -qsuppress=1500-010 -qsuppress=1540-1639 -qsuppress=1540-1088 \
  -lpthread -DALLOW_OPERATOR_NEW_USAGE -c -qmakedep=gcc \
  -MF ../generated/dependencies/loopnode.o.d -o loopnode.o \
  /home/fferrari/jdk8u/hotspot/src/share/vm/opto/loopnode.cpp

-------------

PR Comment: https://git.openjdk.org/jdk8u-dev/pull/600#issuecomment-2547031877