Add compiler options needed by openXL #7447

ishitaR88 · 2024-08-22T19:12:22Z

No description provided.

github-actions

Thank you for supporting the project, and congratulations on your first contribution! A project committer will shortly review your contribution. In the mean time, if you haven't had a chance please skim over the contribution guidelines which all pull requests must adhere to. If the ECA pull request check fails, have a look at the instructions for signing the ECA in the legal considerations section.

If you run into any problems our community will be happy to assist you in any way we can. There are a number of recommended ways to interact with the community. We encourage you to ask questions, or drop by to say hello.

zl-wang · 2024-08-26T13:24:16Z

please provide a proper description of what you are trying to do in this PR

zl-wang · 2024-09-11T19:22:41Z

as for compiler-id, could you make it consistent with Z side (they are switching to OpenXL too, on Z of course)? also, Clang 17.x.x is ambiguous: that 17 refers to OpenXL17, or refers to community Clang version 17?

Detecting OpenXL as XLClang ... this is ambiguous too: we are currently using both xlC and xlclang (i.e. xlC16's xlclang front-end) already. I am not sure if it eventually worked out with this ambiguity, i prefer to staying away from the ambiguity. At least, provide abundant comments for future readers to understand it.

ishitaR88 · 2024-09-12T15:40:18Z

Commit for openXL as Xclang is redundant it's an old change that we don't need anymore so I have removed it.
For the Openxl clang version it is referring to OpenXL17. I have changed the wordings of the comment.

zl-wang · 2024-10-21T19:38:20Z

compiler/compile/OSRData.cpp

@@ -1515,7 +1515,7 @@ bool TR_OSRCompilationData::TR_ScratchBufferInfo::operator==(const TR_ScratchBuf

 #if (defined(TR_HOST_POWER) && defined(TR_TARGET_POWER)                       \
           || defined(TR_HOST_S390) && defined(TR_TARGET_S390))               \
-        && defined(__IBMCPP__)
+        && (defined(__IBMCPP__) || defined(__open_xl__) && defined(__cplusplus))


as raised in the other PR, is defined(__cplusplus) definitely needed here (and more places below)? isn't it automatically defined for c++ files to begin with?

zl-wang · 2024-10-21T19:42:25Z

compiler/env/jittypes.h

@@ -144,7 +144,7 @@ typedef struct TR_InlinedCallSite
      #include <builtins.h>
      #endif /* __cplusplus */
      #define FLUSH_MEMORY(smp) if( smp ) __lwsync();
-   #elif defined(LINUX)
+   #elif defined(LINUX) || defined(__open_xl__)


looks out of place ... shouldn't it be defined in the same clause for AIX? strongly recommend to do that just for readability.

Yes it looks out of place, but the else part is supposed to be for power not sure why there is a check for LINUX in that. But as the definition used for smp is same for open_xl and LINUX macro I didn't put it under another if checking.

zl-wang · 2024-10-21T19:42:59Z

compiler/infra/Annotations.hpp

@@ -64,7 +64,7 @@
 #define OMR_NORETURN _declspec(noreturn)
 #elif defined(__GNUC__)
 #define OMR_NORETURN __attribute__ ((noreturn))
-#elif defined(__IBMCPP__)
+#elif defined(__IBMCPP__) || defined(__open_xl__) && defined(__cplusplus)


zl-wang · 2024-10-21T19:48:28Z

compiler/runtime/OMRRuntimeAssumptions.cpp

@@ -22,7 +22,7 @@
 #include "runtime/OMRRuntimeAssumptions.hpp"
 #include "env/jittypes.h"

-#if defined(__IBMCPP__) && !defined(AIXPPC) && !defined(LINUXPPC)
+#if (defined(__IBMCPP__) || defined(__open_xl__) && defined(__cplusplus)) && !defined(AIXPPC) && !defined(LINUXPPC)


ditto re c++

zl-wang · 2024-10-21T19:53:41Z

compiler/runtime/Runtime.cpp

@@ -23,7 +23,7 @@

 TR_RuntimeHelperTable runtimeHelpers;

-#if (defined(__IBMCPP__) || defined(__IBMC__) && !defined(MVS)) && !defined(LINUXPPC64)
+#if (defined(__IBMCPP__) || defined(__IBMC__) && !defined(MVS)) && !defined(LINUXPPC64) || defined(__open_xl__)


the other comment re MVS applies here too. && and || are used in an unreadable way ... did you really take their precedences into accounts?

As the MVS issue is not introduced by the changes of this PR I will address this in future creating a new PR.

zl-wang · 2024-10-21T19:57:45Z

configure

+    CC="$ac_save_CC $ac_arg"
+    if ac_fn_c_try_compile "$LINENO"; then :
+    ac_cv_prog_cc_c89=$ac_arg
+  fi


indent properly to align with if

zl-wang · 2024-10-21T20:02:09Z

include_core/AtomicSupport.hpp

-#elif defined(__xlC__) /* defined(__GNUC__) */
-#if ((__xlC__ > 0x0d01) || ((__xlC__ == 0x0d01) && (__xlC_ver__ >= 0x00000300))) /* XLC >= 13.1.3 */
+#elif (defined(__xlC__)) /* defined(__GNUC__) */
+#if ((__xlC__ > 0x0d01) || ((__xlC__ == 0x0d01) && (__xlC_ver__ >= 0x00000300))) /* XLC >= 13.1.3 OR OpenXL */


is there a better way to do the same thing? instead of these cryptic numbers

This part of code is not modified to support openxl.

it is commented explicitly there: /* XLC >= 13.1.3 OR OpenXL */ i took it to mean OpenXL build taking true condition on that line?

zl-wang · 2024-10-21T20:04:06Z

include_core/omrcomp.h

@@ -237,7 +237,7 @@ typedef double SYS_FLOAT;
 * xlC11 C++ compiler reportedly supports attributes before function names, but we've only tested xlC12.
 * The C compiler doesn't support it.
 */
-#if defined(__cplusplus) && (__xlC__ >= 0xc00)
+#if defined(__cplusplus) && (__xlC__ >= 0xc00)  


is this relevant to and applies to openxl case?

No. There was some space added by mistake. will remove it.

zl-wang · 2024-10-21T20:09:38Z

port/unix/omrsignal.c

-			Trc_PRT_signal_omrsignal_sig_protect_Exit_long_jumped_back_to_omrsig_protect(fn, fn_arg, handler, handler_arg, flags);
-			return OMRPORT_SIG_EXCEPTION_OCCURRED;
-		}
+		#if !defined(__open_xl__)


this block of code isn't needed for OpenXL build anymore? does it still work as xlC build?

This works as xlc build. But this block of code cause openxl build to fail with the following error on AIX. I think I should add AIX check as well. It may not fail on z/os

But it should not be omitted for openxl. Right now it's just an work around.

Signed-off-by: midronij <[email protected]>

Signed-off-by: Ishita Ray <[email protected]>

This macro will specify if openXL17 clang is used. Signed-off-by: Ishita Ray <[email protected]>

Signed-off-by: midronij <[email protected]>

Signed-off-by: Ishita Ray <[email protected]>

Remove __cplusplus macros that are used along with __open_xl__ macros. Signed-off-by: Ishita Ray <[email protected]>

github-actions bot added arch:power comp:compiler labels Aug 22, 2024

github-actions bot reviewed Aug 22, 2024

View reviewed changes

github-actions bot added the first contribution label Aug 22, 2024

ishitaR88 requested review from babsingh, fjeremic, vijaysun-omr, dsouzai, 0xdaryl and mstoodle as code owners September 11, 2024 18:38

ishitaR88 force-pushed the openxl branch 4 times, most recently from af90606 to 0dcc9e4 Compare September 12, 2024 15:36

ishitaR88 force-pushed the openxl branch 4 times, most recently from b6a07f9 to 61ddfd5 Compare September 13, 2024 19:35

pshipton mentioned this pull request Sep 20, 2024

Adding libs for OpenXL eclipse-openj9/openj9#20045

Open

ishitaR88 force-pushed the openxl branch 5 times, most recently from 1e8f62e to dfa188b Compare September 30, 2024 14:25

ishitaR88 closed this Oct 3, 2024

ishitaR88 force-pushed the openxl branch from dfa188b to d4f0498 Compare October 3, 2024 13:20

ishitaR88 reopened this Oct 3, 2024

ishitaR88 force-pushed the openxl branch from 0c93e1d to 57c8628 Compare October 3, 2024 15:01

ishitaR88 changed the title ~~Initialize variable in PPCTableOfConstants~~ WIP: Add compiler options needed by openXL Oct 3, 2024

ishitaR88 changed the title ~~WIP: Add compiler options needed by openXL~~ Add compiler options needed by openXL Oct 18, 2024

zl-wang suggested changes Oct 21, 2024

View reviewed changes

midronij and others added 5 commits October 24, 2024 12:10

Add __open_xl__ macros to __IBMC__/__IBMCPP__ conditions

f9b6427

Signed-off-by: midronij <[email protected]>

Initialize variable in PPCTableOfConstants

1c12e84

Signed-off-by: Ishita Ray <[email protected]>

Create macro CMAKE_C_COMPILER_IS_OPENXL for openxl

15aa9b2

This macro will specify if openXL17 clang is used. Signed-off-by: Ishita Ray <[email protected]>

Add __open_xl__ macro ifdefs

e4c19f3

Signed-off-by: midronij <[email protected]>

Add macro to openxl specific changes

97c4c6a

Signed-off-by: Ishita Ray <[email protected]>

ishitaR88 force-pushed the openxl branch from 57c8628 to 97c4c6a Compare October 24, 2024 16:11

Remove redundant macros for openxl

921e2eb

Remove __cplusplus macros that are used along with __open_xl__ macros. Signed-off-by: Ishita Ray <[email protected]>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add compiler options needed by openXL #7447

Add compiler options needed by openXL #7447

ishitaR88 commented Aug 22, 2024

github-actions bot left a comment

zl-wang commented Aug 26, 2024

zl-wang commented Sep 11, 2024

ishitaR88 commented Sep 12, 2024

zl-wang Oct 21, 2024

zl-wang Oct 21, 2024

ishitaR88 Oct 24, 2024

zl-wang Oct 21, 2024

zl-wang Oct 21, 2024

zl-wang Oct 21, 2024

ishitaR88 Oct 24, 2024

zl-wang Oct 21, 2024

zl-wang Oct 21, 2024

ishitaR88 Oct 24, 2024 •

edited

Loading

zl-wang Oct 24, 2024

zl-wang Oct 21, 2024

ishitaR88 Oct 24, 2024

zl-wang Oct 21, 2024

ishitaR88 Oct 24, 2024

ishitaR88 Oct 24, 2024

Add compiler options needed by openXL #7447

Are you sure you want to change the base?

Add compiler options needed by openXL #7447

Conversation

ishitaR88 commented Aug 22, 2024

github-actions bot left a comment

Choose a reason for hiding this comment

zl-wang commented Aug 26, 2024

zl-wang commented Sep 11, 2024

ishitaR88 commented Sep 12, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ishitaR88 Oct 24, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ishitaR88 Oct 24, 2024 •

edited

Loading