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

Compiler: -compat-32 flag when building cmo/cma. #896

Merged
merged 2 commits into from May 5, 2017

Conversation

Projects
None yet
5 participants
@hhugo
Copy link
Contributor

commented Nov 8, 2016

-compat-32 is currently only used when linking a bytecode executable.
It would make sense to use -compat-32 for any bytecode object (cmo/cma).

@alainfrisch

This comment has been minimized.

Copy link
Contributor

commented Nov 9, 2016

Good idea, but I'd propose to share this piece of code (and generalize the exception constructor to take a filename). At least Bytepackager and Bytelibrarian could call a function in Bytelink. For sharing with Emitcode, this is less clear.

@mshinwell

This comment has been minimized.

Copy link
Contributor

commented Dec 9, 2016

@hhugo Can you try to finish this one off?

@hhugo hhugo force-pushed the hhugo:compat32 branch 3 times, most recently from e42a1b0 to 055e9c1 Jan 20, 2017

@hhugo

This comment has been minimized.

Copy link
Contributor Author

commented Jan 25, 2017

@alainfrisch, do you prefer the current approach ?

@mshinwell

This comment has been minimized.

Copy link
Contributor

commented Mar 7, 2017

@alainfrisch

This comment has been minimized.

Copy link
Contributor

commented Mar 7, 2017

Yes, I prefer the current approach. But introducing a new module just might be a bit too much. Isn't there a natural place to put this code in an existing module? If a new module must be created, don't forget the standard copyright/license header.

@damiendoligez damiendoligez changed the title Complier: -compat-32 flag when building cmo/cma. Compiler: -compat-32 flag when building cmo/cma. Mar 30, 2017

@hhugo hhugo force-pushed the hhugo:compat32 branch 3 times, most recently from f22104f to 574e1bb Apr 16, 2017

@hhugo

This comment has been minimized.

Copy link
Contributor Author

commented Apr 16, 2017

I've moved the code emitcode.ml and rebased on trunk

Complier: -compat-32 flag when building cmo/cma.
Compiler: refactor -compat-32 support

Update Changes

Complier: more -compat-32 checks in emitcode.

@hhugo hhugo force-pushed the hhugo:compat32 branch from 574e1bb to 6448d31 Apr 25, 2017

@hhugo

This comment has been minimized.

Copy link
Contributor Author

commented Apr 25, 2017

@alainfrisch, would you mind taking another look at this ?

@alainfrisch

This comment has been minimized.

Copy link
Contributor

commented Apr 25, 2017

LGTM. Perhaps add a test case?

@hhugo

This comment has been minimized.

Copy link
Contributor Author

commented May 4, 2017

test case added

@mshinwell mshinwell merged commit b14e6fe into ocaml:trunk May 5, 2017

1 of 2 checks passed

continuous-integration/appveyor/pr AppVeyor build failed
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@shindere

This comment has been minimized.

Copy link
Contributor

commented May 5, 2017

This breaks CI becaue tests/tool-ocamlc-compat32 fails, on several machines,
e.g. linux-32 but not only.

@hhugo can you see why and provide a fix?

@mshinwell

This comment has been minimized.

Copy link
Contributor

commented May 5, 2017

@hhugo is going to work on a fix right now.

@damiendoligez The CI slave for ppc64 appears to have OCaml configured in 32-bit mode. Is that expected?

@mshinwell

This comment has been minimized.

Copy link
Contributor

commented May 5, 2017

(Please don't revert this, we will apply a patch on top)

@shindere

This comment has been minimized.

Copy link
Contributor

commented May 5, 2017

@dra27

This comment has been minimized.

Copy link
Contributor

commented May 6, 2017

The new test is also failing on Windows (I appreciate that AppVeyor results being unavailable for other reasons would have made this difficult to spot).

@hhugo

This comment has been minimized.

Copy link
Contributor Author

commented May 6, 2017

Can one have access to the test result? what the size of integers on windows 64 ?


compile:
printf " ... testing -compat-32"
@$(OCAMLC) -compat-32 -c a.ml 2>&1 | xargs echo ${} > test.result

This comment has been minimized.

Copy link
@dra27

dra27 May 7, 2017

Contributor

On the basis that tests should be as simple as possible, why is the output piped to xargs?

@hhugo

This comment has been minimized.

Copy link
Contributor Author

commented May 7, 2017

@hhugo hhugo deleted the hhugo:compat32 branch Mar 26, 2018

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.