Remove isdigit macro redefs, replace with OMR_ISDIGIT #7711

Deigue · 2025-04-02T16:12:23Z

Changes here remove the redefinition of isdigit() in
the a2e ctype.h headers. In order to fully make this
work functionally, the references/usages of isdigit()
are replaced with OMR_ISDIGIT to properly recognize
ascii digit literals. This is done by conditionally
using __isdigit_a() on z/OS platforms.

This is the cleaned-up changes from #7413 (for historical related comments)

Deigue · 2025-04-02T16:15:42Z

@keithc-ca I unintentionally closed #7413 due to a force push earlier. Don't think I could reopen that, so I created this with the cleaned up suggestions.

I verified the advice by the cpp-team and made the replacements across omr and openj9, and also tested with both XLC and Open XL compilation and confirmed that this builds with the jdk21 zos with no issues for both.

The corresponding changes for OpenJ9 are here: eclipse-openj9/openj9#21555
Which might need to be merged at the same time as this one.

@babsingh Tagging for review/additional feedback, updates from #7413

keithc-ca · 2025-04-02T16:57:36Z

compiler/control/OMROptions.cpp

      {
      int64_t current = 0;
-      while (isdigit(*option))
+      while (__isdigit_a(*option))


I don't think it's reasonable to define _AE_BIMODAL on all platforms, nor is it reasonable to assume that __isdigit_a() exists on all platforms.

I suggest we should introduce helper macros (e.g. omr_isdigit()) which would be suitably defined for each platform and uses of isdigit() would be replaced by uses of omr_isdigit(). The same approach should be applied for toupper(), tolower(), etc.

Sounds good, I can define that in util/a2e/headers/ctype.h I guess.
Is there a __tolower_a() and __toupper_a() similarly available as well? Not able to find documentation around it.

No. omr_isdigit() would be defined in some OMR header file (perhaps after including ctype.h).

keithc-ca · 2025-04-08T14:28:04Z

compiler/control/OMROptions.cpp

            {
            count[i] = atoi(s);
-            while(isdigit(s[0]))
+            while(OMR_ISDIGIT(s[0]))


While you're here, please add the missing space after while; see also lines 4421, 4444 and changes in other files.

keithc-ca · 2025-04-08T14:28:55Z

ddr/include/ddr/std/sstream.hpp

-#include <sstream>
 #endif /* defined(J9ZOS390) */

+#include <sstream>


This was previously not included on z/OS: Have you verified that it is now safe to include?

In the before code, I am seeing #include <sstream> in the J9ZOS390 clause already, inbetween the undef and defines.

That being said, it seems to be safe to include so far while compiling as well.

Sorry, I misread the change. You are correct, sstream was included on all platforms before and should continue to be.

Including ctype.h perhaps should be unconditional:

#include "ddr/config.hpp" #include <ctype.h> #include <sstream>

Likewise in ddr/std/string.hpp and ddr/std/unordered_map.hpp.

keithc-ca · 2025-04-08T14:29:51Z

include_core/omrcomp.h

 #ifndef OMRCOMP_H
 #define OMRCOMP_H

+#define _AE_BIMODAL


I think this should be conditional:

#if defined(J9ZOS390) #define _AE_BIMODAL #endif /* defined(J9ZOS390) */

keithc-ca · 2025-04-08T14:30:44Z

The summary and description should be updated to reflect the new plan.

Deigue · 2025-04-15T16:46:46Z

After having implemented the feedback, with the latest combination of changes from this and eclipse-openj9/openj9#21555 I am seeing the below errors:

/jit/team/gauravc/repos/openj9-openjdk-jdk21-zos/omr/compiler/control/OMROptions.cpp:4418:14: error: use of undeclared identifier '__isdigit_a'
 4418 |          if (OMR_ISDIGIT(s[0]))
      |              ^
/jit/team/gauravc/repos/openj9-openjdk-jdk21-zos/omr/include_core/omrcomp.h:621:24: note: expanded from macro 'OMR_ISDIGIT'
  621 | #define OMR_ISDIGIT(x) __isdigit_a(x)
      |                        ^
/jit/team/gauravc/repos/openj9-openjdk-jdk21-zos/omr/compiler/control/OMROptions.cpp:4421:20: error: use of undeclared identifier '__isdigit_a'
 4421 |             while (OMR_ISDIGIT(s[0]))
      |                    ^
/jit/team/gauravc/repos/openj9-openjdk-jdk21-zos/omr/include_core/omrcomp.h:621:24: note: expanded from macro 'OMR_ISDIGIT'
  621 | #define OMR_ISDIGIT(x) __isdigit_a(x)
      |                        ^
/jit/team/gauravc/repos/openj9-openjdk-jdk21-zos/omr/compiler/control/OMROptions.cpp:4441:14: error: use of undeclared identifier '__isdigit_a'
 4441 |          if (OMR_ISDIGIT(s[0]))
      |              ^
/jit/team/gauravc/repos/openj9-openjdk-jdk21-zos/omr/include_core/omrcomp.h:621:24: note: expanded from macro 'OMR_ISDIGIT'
  621 | #define OMR_ISDIGIT(x) __isdigit_a(x)
      |                        ^
/jit/team/gauravc/repos/openj9-openjdk-jdk21-zos/omr/compiler/control/OMROptions.cpp:4444:20: error: use of undeclared identifier '__isdigit_a'
 4444 |             while (OMR_ISDIGIT(s[0]))
      |                    ^
/jit/team/gauravc/repos/openj9-openjdk-jdk21-zos/omr/include_core/omrcomp.h:621:24: note: expanded from macro 'OMR_ISDIGIT'
  621 | #define OMR_ISDIGIT(x) __isdigit_a(x)
      |                        ^
5 warnings and 9 errors generated.
make[6]: *** [runtime/compiler/CMakeFiles/j9jit.dir/build.make:5974: runtime/compiler/CMakeFiles/j9jit.dir/jit/team/gauravc/repos/openj9-openjdk-jdk21-zos/omr/compiler/control/OMROptions.cpp.o] Error 1

From this it looks like #define AE_BIMODAL might have to be in each file referencing OMR_ISDIGIT() , does that sound right?

keithc-ca · 2025-04-15T20:42:24Z

looks like #define AE_BIMODAL might have to be in each file referencing OMR_ISDIGIT() , does that sound right?

No, that doesn't sound right: That should be contained to the header file that defines OMR_ISDIGIT().

Deigue · 2025-04-16T16:06:06Z

Strange ... I am trying to remove the #if defined(J9ZOS390) in omrcomp.h surrounding the AE_BIMODAL definition to see if it helps, but it would not make sense that part is not working as expected, while the conditional is working around pathing towards and picking the __isdigit_a() call later in that same file.
Also trying to add #include "omrcomp.h" explicitly to see if it makes a difference

If that doesn't work either, then not sure why error: use of undeclared identifier '__isdigit_a' (i.e. isdigit_a() remains undefined) , and will ask around to the compiler team for clarification with the pr changes context.

EDIT: Also trying to include <ctype.h> within omrcomp.h ... noticed that is missing, and might be the reason behind the problem in the first place.

Deigue · 2025-04-21T15:46:49Z

I was able to fix all the compilation problems now, with your recommended suggestions intact.

I was getting the errors from #7711 (comment) for every single file after moving the AE_BIMODAL definition, and kept testing in isolation with the full command.
It seems that having moved the AE_BIMODAL define into the omrcomp.h, it has to be explicitly defined before any direct/indirect inclusion of ctype.h , so that it knows to include the isdigit_a() definition.

Ensuring the omrcomp.h is included first solves the issues, and everything compiles correctly now.

keithc-ca · 2025-04-23T20:32:01Z

ddr/include/ddr/std/sstream.hpp

-#include <sstream>
 #endif /* defined(J9ZOS390) */

+#include <sstream>


Including ctype.h perhaps should be unconditional:

#include "ddr/config.hpp" #include <ctype.h> #include <sstream>

Likewise in ddr/std/string.hpp and ddr/std/unordered_map.hpp.

keithc-ca · 2025-04-23T20:52:29Z

fvtest/omrGtestGlue/omrTest.h

 * So we explicitly include <ctype.h> and undefine the macros for gtest, after gtest we then define back the macros.
 */


This include of ctype.h should also be unconditional, and the stale comment (lines 35-38) removed.

keithc-ca · 2025-04-23T20:54:33Z

util/a2e/headers/ctype.h

                #define isalnum(c)     (_IN_RANGE(c) ? (_ascii_is_tab[c] & _ISALNUM_ASCII) : 0)
                #define isalpha(c)     (_IN_RANGE(c) ? (_ascii_is_tab[c] & _ISALPHA_ASCII) : 0)
                #define iscntrl(c)     (_IN_RANGE(c) ? (_ascii_is_tab[c] & _ISCNTRL_ASCII) : 0)
-                #define isdigit(c)     (_IN_RANGE(c) ? (_ascii_is_tab[c] & _ISDIGIT_ASCII) : 0)


The conditional definition of _AE_BIMODAL should go near the beginning of this file with a surrounding ifdef:

#if !defined(OMR_CTYPE_H) #define OMR_CTYPE_H #if defined(J9ZOS390) #define _AE_BIMODAL #endif /* defined(J9ZOS390) */ // etc. #endif /* !defined(OMR_CTYPE_H) */

Deigue · 2025-04-24T15:10:55Z

@keithc-ca I made some partial changes, to try and confirm the ctype.h change functionality.
However, with the current/latest commit , having the _AE_BIMODAL defined at the top of the a2e ctype.h header, I am still seeing problems with usages of OMR_ISDIGIT() like earlier :

For reference the full command and errors below:

cd /jit/team/gauravc/repos/openj9-openjdk-jdk21-zos/build/zos-s390x-server-release/vm/runtime/compiler && /xlc210/usr/lpp/IBM/cnw/v2r1/openxl/bin/ibm-clang++64  -D__STDC_LIMIT_MACROS -D_ALL_SOURCE -D_OPEN_THREADS=3 -D_XOPEN_SOURCE=600 -Dj9jit_EXPORTS -DBITVECTOR_BIT_NUMBERING_MSB -DBITVECTOR_64BIT -DBUILD_Z_RUNTIME_INSTRUMENTATION -DCOMPRESS_AOT_DATA -DIBM_ATOE -DIPv6_FUNCTION_SUPPORT -DJ9_PROJECT_SPECIFIC -DJ9VM_TIERED_CODE_CACHE -DJ9ZOS390 -DJ9ZOS39064 -DLONGLONG -DMAXMOVE -DOPENJ9_BUILD -DSUPPORTS_THREAD_LOCAL -DTR_HOST_S390 -DTR_HOST_64BIT -DTR_TARGET_S390 -DTR_TARGET_64BIT -DZOS -fstrict-aliasing -mzos-target=ZOSV2R4 -m64 -O3 -fstack-protector -march=arch10 -std=gnu++14 -fasm -fno-rtti -DYYLMAX=1000 -Wuninitialized -mnocsect   -fvisibility=default -fexec-charset=ISO8859-1 -isystem /jit/team/gauravc/repos/openj9-openjdk-jdk21-zos/omr/cmake/modules/platform/os/../../../../util/a2e/headers -I/usr/lpp/hzc/include -I/jit/team/gauravc/repos/openj9-openjdk-jdk21-zos/openj9/runtime/compiler/z -I/jit/team/gauravc/repos/openj9-openjdk-jdk21-zos/openj9/runtime/compiler -I/jit/team/gauravc/repos/openj9-openjdk-jdk21-zos/omr/compiler/z -I/jit/team/gauravc/repos/openj9-openjdk-jdk21-zos/omr/compiler -I/jit/team/gauravc/repos/openj9-openjdk-jdk21-zos/omr -I/jit/team/gauravc/repos/openj9-openjdk-jdk21-zos/omr/thread -I/jit/team/gauravc/repos/openj9-openjdk-jdk21-zos/omr/gc/include -I/jit/team/gauravc/repos/openj9-openjdk-jdk21-zos/openj9/runtime/compiler/.. -I/jit/team/gauravc/repos/openj9-openjdk-jdk21-zos/openj9/runtime/codert_vm -I/jit/team/gauravc/repos/openj9-openjdk-jdk21-zos/openj9/runtime/gc_include -I/jit/team/gauravc/repos/openj9-openjdk-jdk21-zos/openj9/runtime/gc_glue_java -I/jit/team/gauravc/repos/openj9-openjdk-jdk21-zos/openj9/runtime/jit_vm -I/jit/team/gauravc/repos/openj9-openjdk-jdk21-zos/openj9/runtime/nls -I/jit/team/gauravc/repos/openj9-openjdk-jdk21-zos/openj9/runtime/oti -I/jit/team/gauravc/repos/openj9-openjdk-jdk21-zos/openj9/runtime/include -I/jit/team/gauravc/repos/openj9-openjdk-jdk21-zos/openj9/runtime/util -I/jit/team/gauravc/repos/openj9-openjdk-jdk21-zos/build/zos-s390x-server-release/vm/runtime -I/jit/team/gauravc/repos/openj9-openjdk-jdk21-zos/build/zos-s390x-server-release/vm/runtime/oti -I/jit/team/gauravc/repos/openj9-openjdk-jdk21-zos/build/zos-s390x-server-release/vm/runtime/omr -I/jit/team/gauravc/repos/openj9-openjdk-jdk21-zos/build/zos-s390x-server-release/vm/runtime/compiler -I/jit/team/gauravc/repos/openj9-openjdk-jdk21-zos/closedj9/runtime/compiler -I/jit/team/gauravc/repos/openj9-openjdk-jdk21-zos/omr/include_core -I/usr/include -I/usr/lpp/cbclib/include -I/include_core -I/jit/team/gauravc/repos/openj9-openjdk-jdk21-zos/closedj9/runtime/include -I/jit/team/gauravc/repos/openj9-openjdk-jdk21-zos/build/zos-s390x-server-release/vm/runtime/include -I/jit/team/gauravc/repos/openj9-openjdk-jdk21-zos/build/zos-s390x-server-release/vm -I/jit/team/gauravc/repos/openj9-openjdk-jdk21-zos/build/zos-s390x-server-release/vm/runtime/nls -I/jit/team/gauravc/repos/openj9-openjdk-jdk21-zos/omr/util/hashtable/. -I/jit/team/gauravc/repos/openj9-openjdk-jdk21-zos/build/zos-s390x-server-release/vm/runtime/util -I/jit/team/gauravc/repos/openj9-openjdk-jdk21-zos/openj9/runtime/util/.  -o CMakeFiles/j9jit.dir/jit/team/gauravc/repos/openj9-openjdk-jdk21-zos/omr/compiler/control/OMROptions.cpp.o -c /jit/team/gauravc/repos/openj9-openjdk-jdk21-zos/omr/compiler/control/OMROptions.cpp

//errors:
/jit/team/gauravc/repos/openj9-openjdk-jdk21-zos/omr/compiler/control/OMROptions.cpp:1379:14: error: use of undeclared identifier '__isdigit_a'
 1379 |       while (OMR_ISDIGIT(*option))
      |              ^
/jit/team/gauravc/repos/openj9-openjdk-jdk21-zos/omr/include_core/omrcomp.h:617:24: note: expanded from macro 'OMR_ISDIGIT'
  617 | #define OMR_ISDIGIT(x) __isdigit_a(x)
      |                        ^
/jit/team/gauravc/repos/openj9-openjdk-jdk21-zos/omr/compiler/control/OMROptions.cpp:3429:20: error: use of undeclared identifier '__isdigit_a'
 3429 |             while (OMR_ISDIGIT(*options))
      |                    ^
/jit/team/gauravc/repos/openj9-openjdk-jdk21-zos/omr/include_core/omrcomp.h:617:24: note: expanded from macro 'OMR_ISDIGIT'
  617 | #define OMR_ISDIGIT(x) __isdigit_a(x)
      |                        ^
/jit/team/gauravc/repos/openj9-openjdk-jdk21-zos/omr/compiler/control/OMROptions.cpp:3446:23: error: use of undeclared identifier '__isdigit_a'
 3446 |                while (OMR_ISDIGIT(*options))
      |                       ^
/jit/team/gauravc/repos/openj9-openjdk-jdk21-zos/omr/include_core/omrcomp.h:617:24: note: expanded from macro 'OMR_ISDIGIT'
  617 | #define OMR_ISDIGIT(x) __isdigit_a(x)
      |                        ^
/jit/team/gauravc/repos/openj9-openjdk-jdk21-zos/omr/compiler/control/OMROptions.cpp:4393:14: error: use of undeclared identifier '__isdigit_a'
 4393 |          if (OMR_ISDIGIT(s[0]))
      |              ^
/jit/team/gauravc/repos/openj9-openjdk-jdk21-zos/omr/include_core/omrcomp.h:617:24: note: expanded from macro 'OMR_ISDIGIT'
  617 | #define OMR_ISDIGIT(x) __isdigit_a(x)
      |                        ^
/jit/team/gauravc/repos/openj9-openjdk-jdk21-zos/omr/compiler/control/OMROptions.cpp:4396:20: error: use of undeclared identifier '__isdigit_a'
 4396 |             while (OMR_ISDIGIT(s[0]))
      |                    ^
/jit/team/gauravc/repos/openj9-openjdk-jdk21-zos/omr/include_core/omrcomp.h:617:24: note: expanded from macro 'OMR_ISDIGIT'
  617 | #define OMR_ISDIGIT(x) __isdigit_a(x)
      |                        ^
/jit/team/gauravc/repos/openj9-openjdk-jdk21-zos/omr/compiler/control/OMROptions.cpp:4418:14: error: use of undeclared identifier '__isdigit_a'
 4418 |          if (OMR_ISDIGIT(s[0]))
      |              ^
/jit/team/gauravc/repos/openj9-openjdk-jdk21-zos/omr/include_core/omrcomp.h:617:24: note: expanded from macro 'OMR_ISDIGIT'
  617 | #define OMR_ISDIGIT(x) __isdigit_a(x)
      |                        ^
/jit/team/gauravc/repos/openj9-openjdk-jdk21-zos/omr/compiler/control/OMROptions.cpp:4421:20: error: use of undeclared identifier '__isdigit_a'
 4421 |             while (OMR_ISDIGIT(s[0]))
      |                    ^
/jit/team/gauravc/repos/openj9-openjdk-jdk21-zos/omr/include_core/omrcomp.h:617:24: note: expanded from macro 'OMR_ISDIGIT'
  617 | #define OMR_ISDIGIT(x) __isdigit_a(x)
      |                        ^
/jit/team/gauravc/repos/openj9-openjdk-jdk21-zos/omr/compiler/control/OMROptions.cpp:4441:14: error: use of undeclared identifier '__isdigit_a'
 4441 |          if (OMR_ISDIGIT(s[0]))
      |              ^
/jit/team/gauravc/repos/openj9-openjdk-jdk21-zos/omr/include_core/omrcomp.h:617:24: note: expanded from macro 'OMR_ISDIGIT'
  617 | #define OMR_ISDIGIT(x) __isdigit_a(x)
      |                        ^
/jit/team/gauravc/repos/openj9-openjdk-jdk21-zos/omr/compiler/control/OMROptions.cpp:4444:20: error: use of undeclared identifier '__isdigit_a'
 4444 |             while (OMR_ISDIGIT(s[0]))
      |                    ^
/jit/team/gauravc/repos/openj9-openjdk-jdk21-zos/omr/include_core/omrcomp.h:617:24: note: expanded from macro 'OMR_ISDIGIT'
  617 | #define OMR_ISDIGIT(x) __isdigit_a(x)

I tried moving around the define of AE_BIMODAL itself a bit around in the file itself, but I don't know if that is the problem.
With the current state of the code, there is something that is still making it such that when the macro is encountered, the function isn't defined.

I am having difficulty finding out why isdigit_a is undefined, as the explanation behind the initialization you mentioned yesterday made sense. Is there something else that is prompting the omrcomp.h itself to load its contents/usages prior to ctype.h itself ?

Deigue · 2025-04-28T18:10:07Z

Made the changes to address the above feedback. Tested with the commandline approach using -D_AE_BIMODAL to apply it consistently everywhere and it seems to fix up all the compilation issues that were being seen earlier.

keithc-ca · 2025-04-28T20:29:02Z

cmake/modules/platform/os/zos.cmake

 #############################################################################

 list(APPEND OMR_PLATFORM_DEFINITIONS
+	-D_AE_BIMODAL


Should this be contributed by openxl.cmake instead? Or will this work with xlc as well?

This works with xlc as well. I checked this with the compiler team and had to make sure it compiles with both using AE_BIMODAL , because after removing the redefinitions from ctype.h , it broke the xlc builds as well. So adding it here was intentional.

keithc-ca · 2025-04-28T20:30:11Z

fvtest/omrGtestGlue/omrTest.h

 static int iconv_init_static_variable = iconv_initialization();
 #endif /* defined(J9ZOS390) && !defined(OMR_EBCDIC) */




Please also delete this blank line (we don't need more than one blank line in a row).

keithc-ca · 2025-04-28T20:31:22Z

util/a2e/headers/ctype.h

+#if defined(J9ZOS390)
+#define _AE_BIMODAL
+#endif /* defined(J9ZOS390) */


This is at best redundant (it should be defined by the make system) or may be viewed by the compiler as a redefinition (error).

Mistake from my side, should have removed it earlier.

Deigue · 2025-04-30T15:05:45Z

@keithc-ca Both this and eclipse-openj9/openj9#21555 are ready for review, with latest feedback in place/tested.

babsingh · 2025-05-06T16:46:23Z

jenkins build all

babsingh · 2025-05-06T18:22:54Z

@Deigue Can you look at the failures in the zOS PR build?

babsingh · 2025-05-07T15:03:53Z

failures look like infra issues ... restarting PR builds

jenkins build zos,riscv

Deigue · 2025-05-07T15:25:23Z

I am seeing that bunch of the console output messages earlier were indicative of OMR_ISDIGIT not working as planned within OMROptions.cpp , where it has multiple usages, and I assume is used by ctest for omr jenkins as well.
Will try to proactively reproduce these errors if I can by attempting a standalone omr zos build with xlc locally, in the past I did see this in one occurence, when AE_BIMODAL was not applying correctly to some files.

babsingh · 2025-05-08T15:58:03Z

jenkins build zos(compile:'VERBOSE=1')

Deigue · 2025-05-09T17:17:13Z

@babsingh Is there a reason why locally I am only seeing 15 tests with ctest -V ?

15/15 Test #15: gctest ...........................   Passed    0.43 sec

100% tests passed, 0 tests failed out of 15

Total Test time (real) = 122.40 sec

While in the verbose console log these is 28 tests? Is this dependant on the way cmake configures the build at the start?

Looks like the OMROptions.cpp command/compilation looks ok, so there is something else may be the problem?

[2025-05-08T16:02:33.357Z] [ 25%] Building CXX object fvtest/compilertest/CMakeFiles/testcompiler.dir/__/__/compiler/control/OMROptions.cpp.o
[2025-05-08T16:02:33.357Z] cd /u/user1/workspace/Build/build/fvtest/compilertest && /bin/xlc   -D__STDC_LIMIT_MACROS -D_AE_BIMODAL -D_ALL_SOURCE -D_OPEN_THREADS=3 -D_XOPEN_SOURCE=600 -DBITVECTOR_BIT_NUMBERING_MSB -DBITVECTOR_64BIT -DJITTEST -DJ9ZOS390 -DJ9ZOS39064 -DLONGLONG -DOMR_EBCDIC -DPROD_WITH_ASSUMES -DSUPPORTS_THREAD_LOCAL -DTEST_PROJECT_SPECIFIC -DTR_HOST_S390 -DTR_HOST_64BIT -DTR_TARGET_S390 -DTR_TARGET_64BIT -DZOS -I/u/user1/workspace/Build/fvtest/compilertest/z -I/u/user1/workspace/Build/fvtest/compilertest -I/u/user1/workspace/Build/compiler/z -I/u/user1/workspace/Build/compiler -I/u/user1/workspace/Build -I/u/user1/workspace/Build/fvtest -I/u/user1/workspace/Build/include_core -I/u/user1/workspace/Build/build  "-Wc,xplink" "-Wc,rostring" "-Wc,FLOAT(IEEE,FOLD,AFP)" "-Wc,enum(4)" "-Wa,goff" "-Wc,NOANSIALIAS" "-Wc,TARGET(ZOSV2R3)" -Wc,lp64 "-Wa,SYSPARM(BIT64)" -+ "-Wc,ARCH(10)" "-Wc,TUNE(12)" "-Wl,compat=ZOSV2R3" "-Wc,langlvl(extended)" -qlanglvl=extended0x -qasm -DYYLMAX=1000 -Wa,asa -Wc,EXH -qhaltonmsg=CCN6102 -qnocsect   -o CMakeFiles/testcompiler.dir/__/__/compiler/control/OMROptions.cpp.o -c /u/user1/workspace/Build/compiler/control/OMROptions.cpp

I can see both D_AE_BIMODAL and -DJ9ZOS390 set, which should ensure the correct __isdigit_a() is coming into play with the macro.

babsingh · 2025-05-09T17:20:23Z

While in the verbose console log these is 28 tests? Is this dependant on the way cmake configures the build at the start?

There may be conditional restrictions based on the setup and environment. These can usually be identified by reviewing the test source code and build configuration files.

babsingh · 2025-05-09T17:23:21Z

Looks like the OMROptions.cpp command/compilation looks ok, so there is something else may be the problem?

Unsure since they are JIT failures. @0xdaryl Is there a JIT member who can help @Deigue investigate these failures?

12:23:02  The following tests FAILED:
12:23:02  	  3 - conditionals_example_as_test (Failed)
12:23:02  	  5 - iterfib_example_as_test (Failed)
12:23:02  	  6 - nestedloop_example_as_test (Failed)
12:23:02  	  7 - pow2_example_as_test (Failed)
12:23:02  	  8 - simple_example_as_test (Failed)
12:23:02  	  9 - worklist_example_as_test (Failed)
12:23:02  	 10 - power_example_as_test (Failed)
12:23:02  	 24 - JitBuilderTest (OTHER_FAULT)
12:23:02  	 25 - CompilerTest (OTHER_FAULT)
12:23:02  	 26 - triltest (OTHER_FAULT)
12:23:02  	 27 - comptest (OTHER_FAULT)
12:23:02  	 28 - compunittest (OTHER_FAULT)

Deigue · 2025-05-13T15:06:05Z

I was chatting with @Spencer-Comin for some guidance on this,
The way it was running was off locally, so I ran it with the same configure and build commands, and also adding the current build directory to LIBPATH to ensure the ctest run afterwards dont error out,

cmake -Wdev -C../cmake/caches/Travis.cmake -DCMAKE_C_COMPILER=/bin/c89 -DCMAKE_CXX_COMPILER=/bin/xlc -DOMR_DDR=OFF -DOMR_THR_FORK_SUPPORT=0 ..

make -j4 VERBOSE=1

However this has got my build to fail/error before the eclipse-omr/pr/zos_390_64 build at compilertest

cd /jit/team/gauravc/repos/xlc/openj9-openjdk-jdk21-zos/omr/build/fvtest/compilertest && /rsusr/openj9_resources/bin/cmake -E cmake_link_script CMakeFiles/compilertest.dir/link.txt --verbose=1

Attached linker errors associated with local build:
linker-compilertest.txt

The full commands look the same.
I was trying to look at the nm ../../libtestcompiler.a output, but cannot make sense out of it.

I do have OMROptions.cpp compiling with this configure though, and the full command does seem to be good. Wanted to try to get it at the same position as the official eclipse build.
The <JIT: fatal error: invalid command line> originate from rossa.cpp in openj9 , that is parsing of the JIT Options ... so I am wondering, @babsingh Do you think the openj9 changes will be needed too? I thought openj9 wouldnt be used at all in terms of jenkins builds.

babsingh · 2025-05-13T15:17:18Z

The <JIT: fatal error: invalid command line> originate from rossa.cpp in openj9 , that is parsing of the JIT Options ... so I am wondering, @babsingh Do you think the openj9 changes will be needed too?

No, these errors also exist in OMR - see CompileMethod.cpp.

Changes here remove the redefinition of isdigit() in the a2e ctype.h headers. In order to fully make this work functionally, the references/usages of isdigit() are replaced with OMR_ISDIGIT to properly recognize ascii digit literals. This is done by conditionally using __isdigit_a() on z/OS platforms. Signed-off-by: Gaurav Chaudhari <gaurav.chaudhari@ibm.com>

babsingh · 2025-05-15T16:14:09Z

jenkins build zos(compile:'VERBOSE=1')

Deigue · 2025-05-20T17:46:02Z

Are we good for merging the pair of PR's now, as the builds have passed?

The only change was made to omrcomp.h as below:

#if defined(J9ZOS390) && !defined(OMR_EBCDIC)

I added the !defined(OMR_EBCDIC) , as this was the reason why the OMR only builds were failing. With OMR standalone, the builds were running in EBCDIC mode, and in these scenarios it should not be using the ascii version, i.e. isdigit_a()

The extension repos compilation along with OMR/OpenJ9 remains to be fine.

babsingh · 2025-05-20T21:59:06Z

jenkins build all

Deigue requested review from 0xdaryl, dsouzai and mstoodle as code owners April 2, 2025 16:12

Deigue force-pushed the openxl-macros branch from 28f6846 to 9597ab4 Compare April 2, 2025 16:20

keithc-ca suggested changes Apr 2, 2025

View reviewed changes

keithc-ca mentioned this pull request Apr 2, 2025

Replace isdigit with OMR_ISDIGIT eclipse-openj9/openj9#21555

Merged

Deigue force-pushed the openxl-macros branch from 9597ab4 to 47e7fa0 Compare April 7, 2025 19:11

Deigue marked this pull request as draft April 7, 2025 19:11

keithc-ca suggested changes Apr 8, 2025

View reviewed changes

Deigue changed the title ~~Remove isdigit macro redefs, replace with __isdigit_a~~ Remove isdigit macro redefs, replace with OMR_ISDIGIT Apr 8, 2025

Deigue force-pushed the openxl-macros branch 2 times, most recently from 68797ef to 3f88031 Compare April 15, 2025 14:24

Deigue force-pushed the openxl-macros branch from 3f88031 to 733f9a8 Compare April 21, 2025 15:43

Deigue marked this pull request as ready for review April 21, 2025 15:48

Deigue requested a review from keithc-ca April 22, 2025 15:07

keithc-ca suggested changes Apr 23, 2025

View reviewed changes

Deigue force-pushed the openxl-macros branch from 733f9a8 to 3da3638 Compare April 24, 2025 15:03

Deigue marked this pull request as draft April 24, 2025 15:11

Deigue force-pushed the openxl-macros branch 2 times, most recently from a018747 to c66161e Compare April 28, 2025 18:07

Deigue requested a review from keithc-ca April 28, 2025 18:10

keithc-ca suggested changes Apr 28, 2025

View reviewed changes

Deigue force-pushed the openxl-macros branch from c66161e to 9abb877 Compare April 29, 2025 15:09

Deigue marked this pull request as ready for review April 29, 2025 15:09

Deigue requested a review from keithc-ca April 29, 2025 15:09

keithc-ca approved these changes Apr 30, 2025

View reviewed changes

Deigue force-pushed the openxl-macros branch from 9abb877 to f4e8166 Compare May 15, 2025 14:58

Deigue force-pushed the openxl-macros branch from f4e8166 to 9d64016 Compare May 15, 2025 14:59

babsingh approved these changes May 21, 2025

View reviewed changes

babsingh merged commit c18333a into eclipse-omr:master May 21, 2025
13 checks passed

babsingh mentioned this pull request May 22, 2025

Revert "Remove isdigit macro redefs, replace with OMR_ISDIGIT" #7760

Merged

		* So we explicitly include <ctype.h> and undefine the macros for gtest, after gtest we then define back the macros.
		*/

		static int iconv_init_static_variable = iconv_initialization();
		#endif /* defined(J9ZOS390) && !defined(OMR_EBCDIC) */

Remove isdigit macro redefs, replace with OMR_ISDIGIT #7711

Remove isdigit macro redefs, replace with OMR_ISDIGIT #7711

Uh oh!

Conversation

Deigue commented Apr 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Deigue commented Apr 2, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

keithc-ca commented Apr 8, 2025

Uh oh!

Deigue commented Apr 15, 2025

Uh oh!

keithc-ca commented Apr 15, 2025

Uh oh!

Deigue commented Apr 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Deigue commented Apr 21, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Deigue commented Apr 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Deigue commented Apr 28, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Deigue commented Apr 30, 2025

Uh oh!

babsingh commented May 6, 2025

Uh oh!

babsingh commented May 6, 2025

Uh oh!

babsingh commented May 7, 2025

Uh oh!

Deigue commented May 7, 2025

Uh oh!

babsingh commented May 8, 2025

Uh oh!

Deigue commented May 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

babsingh commented May 9, 2025

Uh oh!

babsingh commented May 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Deigue commented May 13, 2025

Deigue commented Apr 2, 2025 •

edited

Loading

Deigue commented Apr 16, 2025 •

edited

Loading

Deigue commented Apr 24, 2025 •

edited

Loading

Deigue commented May 9, 2025 •

edited

Loading

babsingh commented May 9, 2025 •

edited

Loading