Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Restore support for bytecode target XLC/AIX/Power #2295

Merged
merged 1 commit into from Mar 21, 2019

Conversation

Projects
None yet
4 participants
@ksromanov
Copy link
Contributor

commented Mar 6, 2019

This patch restores support for bytecode target using IBM XLC compiler on AIX. Both 64-bit and 32-bit builds work. The functionality was affected during transition to autoconf - PR #2139.

Original xlc PR - #1130.

@ksromanov

This comment has been minimized.

Copy link
Contributor Author

commented Mar 6, 2019

Please review. @dra27 is especially welcome!
I have not included generated configure script yet for clarity.

Show resolved Hide resolved configure.ac Outdated

@ksromanov ksromanov changed the title Restore support for bytecode target XLC/AIX/Power [NOT-FOR-MERGE-YET] Restore support for bytecode target XLC/AIX/Power Mar 6, 2019

@mshinwell mshinwell changed the title [NOT-FOR-MERGE-YET] Restore support for bytecode target XLC/AIX/Power Restore support for bytecode target XLC/AIX/Power Mar 7, 2019

@ksromanov

This comment has been minimized.

Copy link
Contributor Author

commented Mar 12, 2019

I would like to write somewhere short note on how to use xlc on AIX/Power:

For 32-bit build

CC=xlc ./configure
make clean; make world

For 64-bit build

OBJECT_MODE=64 CC=xlc ./configure
make clean; OBJECT_MODE=64 make world

It important to set OBJECT_MODE=64 for make world as well, or resulting ocamlrun segfaults.

What is the best place to put it?

@dra27

This comment has been minimized.

Copy link
Contributor

commented Mar 12, 2019

I think it could be added in the PREQUISITES section if INSTALL.adoc (@gasche?)

Note that it should be ./configure CC=xlc (i.e. CC after ./configure, not before)

I’m not familiar with the platform or compiler - what is OBJECT_MODE responsible for doing (precisely)?

@dra27

This comment has been minimized.

Copy link
Contributor

commented Mar 12, 2019

Ah, I found some docs - is there a flag -q64 for xlc which does the same thing? It would seem better to inject that via CFLAGS (which may require another fix to configure.ac IIRC)

@ksromanov

This comment has been minimized.

Copy link
Contributor Author

commented Mar 12, 2019

OBJECT_MODE also enables 64-bit mode for linker (bind64) and archiver (ar). I personally don't know, how to pass -X 64 to ar via configure parameters. So, we just use OBJECT_MODE=64.

I just tried ./configure CC=xlc - it works well.

@dra27
Copy link
Contributor

left a comment

Two small changes, but otherwise LGTM, thanks!

@@ -395,6 +395,7 @@ AS_CASE([$host],
[
libext=lib
AR=""; RANLIB=echo; RANLIBCMD=""
CPP="$CC -nologo -EP"

This comment has been minimized.

Copy link
@dra27

dra27 Mar 13, 2019

Contributor

This line is not required

This comment has been minimized.

Copy link
@ksromanov

ksromanov Mar 13, 2019

Author Contributor

Removed.

AS_CASE([$host],
[*-*-mingw32|*-pc-windows], [cc_has_debug_prefix_map=false],
AS_CASE([$CC,$host],
[*,*-*-mingw32|*,*-pc-windows], [cc_has_debug_prefix_map=false],

This comment has been minimized.

Copy link
@dra27

dra27 Mar 13, 2019

Contributor

Please could this be split into two lines - I think it should either be one regexp covering all three platforms (which would be too long a line) or three separate regexps all setting cc_has_debug_prefix_map=false, but not a mix of the two!

This comment has been minimized.

Copy link
@ksromanov

ksromanov Mar 13, 2019

Author Contributor

Done.

Konstantin Romanov
@ksromanov

This comment has been minimized.

Copy link
Contributor Author

commented Mar 21, 2019

Rebased on top of current trunk f7f5f21

All @dra27 comments were addressed.

@shindere shindere merged commit f1a1347 into ocaml:trunk Mar 21, 2019

1 of 2 checks passed

continuous-integration/appveyor/pr Waiting for AppVeyor build to complete
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

shindere added a commit that referenced this pull request Mar 21, 2019

@shindere

This comment has been minimized.

Copy link
Contributor

commented Mar 21, 2019

@ksromanov ksromanov deleted the ksromanov:aix_XLC branch Mar 21, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.