Skip to content

Change the generated code model to Large on Windows 64 bit #7489

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

Merged
merged 1 commit into from
Feb 25, 2017
Merged

Change the generated code model to Large on Windows 64 bit #7489

merged 1 commit into from
Feb 25, 2017

Conversation

hughbe
Copy link
Contributor

@hughbe hughbe commented Feb 15, 2017

See discussion in #3841

Before this, generated code referenced 32 bit addresses, causing segfaults when running it on Windows 64 bit.

@hughbe
Copy link
Contributor Author

hughbe commented Feb 15, 2017

@swift-ci please smoke test

// On Windows 64 bit, dlls are loaded above the max address for 32 bits.
// This means that a default CodeModel causes generate code to segfault
// when run.
if (Triple.isArch64Bit() && Triple.isOSWindows())
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should Cygwin-32 continue using the large model too?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's unclear, I'm taking this from a comment in that Pr:

Cygwin should use large memory model as default.(This may be changed
if someone ports to 32bit)

I think for now specialising this to 64 bit, which is the only one that's been tested, is the right way to go.

Logically speaking, I'm not sure how referencing a 32 bit address on a 32 bit architecture would be bad, but I can't test this

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tinysun212, any insights to offer?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@compnerd if no insights are to be offered

@hughbe
Copy link
Contributor Author

hughbe commented Feb 18, 2017

@swift-ci please smoke test

@tinysun212
Copy link
Contributor

It seems that nobody ports to 32bit Cygwin.
This patch will work also on MinGW 64bit and looks good to me.
@compnerd, How about on the Windows-Itanium platform?

@erg
Copy link
Contributor

erg commented Feb 24, 2017

LGTM

@hughbe hughbe merged commit ba92028 into swiftlang:master Feb 25, 2017
@hughbe hughbe deleted the codemodel-64bit branch February 25, 2017 01:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants