Create macro to identify openXL compiler #7480

ishitaR88 · 2024-10-03T18:27:43Z

Added macro CMAKE_C_COMPILER_IS_OPENXL that will be used to add compiler options for openXL

zl-wang · 2024-10-04T01:24:26Z

cmake/modules/OmrDetectSystemInformation.cmake

-				# TODO we don't actually have a clang config
-				# just use GNU config
-				set(_OMR_TOOLCONFIG "gnu")
+			elseif(CMAKE_C_COMPILER_ID MATCHES "^Clang$")


Z system will use OpenXL compiler too. Have you make sure either of the two below?

Z system will not use Clang compiler ID for that expected switch; or

this config setting is applicable to them as well.

@r30shah please comment ...

Yes, I believe the COMPILER_ID on Z will be IBMClang and TOOLCONFIG will be set to openxl. I would like @Deigue to confirm this.

I didn't consider that the COMPILER_ID can be different for Z system.

you had better get together with @Deigue
either, both can use the same ID, if we can;
or, AIX uses a more differentiating and meaningful ID, rather than this very generic "Clang"

I can't find any existing toolconfig for "openxl" that's why I used "gnu" for now.

I've made the compiler id related changes here: #7319 ,
also will be moving the "IBMClang$" somewhere alongside the xlc clause so that all that compiler id variations can navigate to openxl toolchain easily from one spot based on Keiths feedback.

From what I remember, I was seeing zOS as the compiler ID while running locally, but when trying a Open XL build on Jenkins for z/OS, I was seeing the compiler id come out as "IBMClang" ...

In any case, I did create my own openxl.cmake toolconfig in the PR above.

@Deigue Can this openxl.cmake be used for AIX as well?

I would say yes, because there is a section in the file I left behind that says
if(OMR_OS_AIX) , that leaves the default state of setup/configs, I suspect from xlc.cmake .

So you would be able to use that section, but you may need to adapt the AIX configs to be whatever you need to make things work.

keithc-ca · 2024-10-04T16:16:10Z

cmake/modules/OmrDetectSystemInformation.cmake

+                                # OpenXL17 uses CMAKE_C_COMPILER_ID "Clang"
+                                set(_OMR_TOOLCONFIG "gnu")
+                                set(CMAKE_C_COMPILER_IS_OPENXL TRUE CACHE BOOL "OpenXL is the C compiler")


Lines should be indented with tabs (like the rest of this file).

keithc-ca · 2024-10-04T16:16:57Z

compiler/p/codegen/PPCTableOfConstants.cpp

@@ -499,7 +499,7 @@ int32_t TR_PPCTableOfConstants::lookUp(TR::SymbolReference *symRef, TR::CodeGene
   int8_t           local_buffer[1024];
   int8_t          *name = local_buffer;
   bool             isAddr = false;
-   intptr_t        myTag;
+   intptr_t        myTag=0;


Please add a space both sides of = (like the lines above).

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

keithc-ca · 2024-10-04T16:48:32Z

compiler/p/codegen/PPCTableOfConstants.cpp

@@ -499,7 +499,7 @@ int32_t TR_PPCTableOfConstants::lookUp(TR::SymbolReference *symRef, TR::CodeGene
   int8_t           local_buffer[1024];
   int8_t          *name = local_buffer;
   bool             isAddr = false;
-   intptr_t        myTag;
+   intptr_t        myTag = 0;


This change seems unrelated to the title of this pull request; perhaps it should be proposed separately?

It is a good change: It just should just be independent of build configurations.

Yes. We need this change. We will add this change through another PR.

keithc-ca · 2024-10-08T17:52:28Z

cmake/modules/OmrDetectSystemInformation.cmake

+				if(CMAKE_C_COMPILER_IS_OPENXL)
+					set(OMR_ENV_OPENXL 1)
+					set(ENV{OMR_ENV_OPENXL} ${OMR_ENV_OPENXL})
+				else()
+					set(OMR_ENV_OPENXL 0)
+					set(ENV{OMR_ENV_OPENXL} ${OMR_ENV_OPENXL})
+				endif()


Why would we want this? Won't CMAKE_C_COMPILER_IS_OPENXL be sufficient?

That environment variable is for Makefiles to check if the compiler is openxl. I used that env variable instead of adding openxl in defaults.yml

Which makefiles? With cmake, we don't write makefiles (cmake might even use another generator, e.g. ninja).
I don't see any references to OMR_ENV_OPENXL, neither in cmakefiles nor the environment; perhaps it would help if you provided a sample of how you imagine they would be used.

Following is an example where the environment variable is used in openj9/runtime/makelib/targets.mk.aix.inc.ftl

ifeq ($(OMR_ENV_OPENXL),1) # openxl options CXXFLAGS += -std=c++11 -qarch=ppc -fno-strict-aliasing -fstack-protector else CXXFLAGS += -qlanglvl=extended0x -qarch=ppc -qalias=noansi -qxflag=LTOL:LTOL0 -qsuppress=1506-1108 -qstackprotect endif

More use of this env variable can be found in this PR eclipse-openj9/openj9#20045

Those makefiles are not used when cmake is being used.

Yes. I used this CMAKE variable to let the makefiles know that the compiler is openxl. The other way of doing it may be by setting the compiler in defaults.yml

My point is that when using cmake, files like openj9/runtime/gc_glue_java/configure_includes/configure_aix_ppc.mk are not used, so the reference there (in eclipse-openj9/openj9#20045) is not helpful.

these PRs were intended to make changes which allows builds by all available (and being used) compilers (i.e. xlC/xlclang/OpenXL), through either cmake or UMA. so, we need to keep these changes anyway.

Yes, we would like UMA builds to be possible with OpenXL (although I consider that a bonus, not a necessity), but setting an environment variable for cmake can't benefit UMA builds, so the question remains: why is the variable being set (in cmake)?

midronij · 2024-11-27T16:20:07Z

Ishita will be away for the foreseeable future, so I will be taking over this contribution here: #7561
@zl-wang could you please close this PR when you have a moment?

zl-wang · 2024-11-27T16:48:49Z

i remembered she had 3 PRs (2 OMR and 1 OpenJ9). @midronij have you squashed that 2 OMR into your single one? also, you might need a separate one to init myTag to zero (in order to avoid OpenXL generating garbage code).

midronij · 2024-11-27T16:58:22Z

This PR is included in my new OMR PR, yes. Specifically in this commit: 201f3c8. The fix to initialize myTag is also included (a1c05d4), but I can separate it out into another PR if that would be better

dsouzai · 2024-11-27T19:04:03Z

Closing as requested.

and eclipse-omr#7480 Signed-off-by: midronij <[email protected]>

github-actions bot added arch:power build / configure cmake comp:compiler labels Oct 3, 2024

zl-wang reviewed Oct 4, 2024

View reviewed changes

keithc-ca reviewed Oct 4, 2024

View reviewed changes

Create macro CMAKE_C_COMPILER_IS_OPENXL for openxl

002c25e

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

ishitaR88 force-pushed the openxl_part1 branch from b133569 to 4fee25b Compare October 4, 2024 16:38

keithc-ca reviewed Oct 7, 2024

View reviewed changes

keithc-ca reviewed Oct 8, 2024

View reviewed changes

ishitaR88 force-pushed the openxl_part1 branch from 4fee25b to 002c25e Compare November 26, 2024 20:54

midronij mentioned this pull request Nov 27, 2024

Modifications to ensure compatibility with OpenXL and xlC on AIX #7561

Merged

dsouzai closed this Nov 27, 2024

midronij added a commit to midronij/omr that referenced this pull request Nov 28, 2024

Requested changes from eclipse-omr#7447

7615492

and eclipse-omr#7480 Signed-off-by: midronij <[email protected]>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Create macro to identify openXL compiler #7480

Create macro to identify openXL compiler #7480

ishitaR88 commented Oct 3, 2024

zl-wang Oct 4, 2024

zl-wang Oct 4, 2024

r30shah Oct 4, 2024

ishitaR88 Oct 4, 2024

zl-wang Oct 4, 2024

ishitaR88 Oct 4, 2024

Deigue Oct 4, 2024 •

edited

Loading

ishitaR88 Oct 4, 2024

Deigue Oct 4, 2024

keithc-ca Oct 4, 2024

ishitaR88 Oct 4, 2024

keithc-ca Oct 4, 2024

ishitaR88 Oct 4, 2024

keithc-ca Oct 4, 2024

ishitaR88 Nov 26, 2024

keithc-ca Nov 26, 2024

ishitaR88 Nov 26, 2024

keithc-ca Oct 8, 2024

ishitaR88 Oct 8, 2024

keithc-ca Oct 8, 2024

ishitaR88 Nov 26, 2024

keithc-ca Nov 26, 2024

ishitaR88 Nov 26, 2024

keithc-ca Nov 26, 2024

zl-wang Nov 27, 2024

keithc-ca Nov 27, 2024

midronij commented Nov 27, 2024

zl-wang commented Nov 27, 2024 •

edited

Loading

midronij commented Nov 27, 2024 •

edited

Loading

dsouzai commented Nov 27, 2024

Create macro to identify openXL compiler #7480

Create macro to identify openXL compiler #7480

Conversation

ishitaR88 commented Oct 3, 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

Deigue Oct 4, 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

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

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

midronij commented Nov 27, 2024

zl-wang commented Nov 27, 2024 • edited Loading

midronij commented Nov 27, 2024 • edited Loading

dsouzai commented Nov 27, 2024

Deigue Oct 4, 2024 •

edited

Loading

zl-wang commented Nov 27, 2024 •

edited

Loading

midronij commented Nov 27, 2024 •

edited

Loading