Revert "explicitly specify model for relocatable variables" #1368
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This reverts commit 103b25f. That breaks compilation on at least LoongArch and IA-64 with latest GCC because LoongArch GCC does not recognize "small" and IA-64 GCC accepts an identifier (instead of string literal). Per the ChangeLog it seems the change was for "supporting binaries larger than 2 GiB" but I cannot see how it will work reliably. If a binary will be larger than the limit of the default code model, the entire binary (i.e. each relocatable object file that would consist the binary) must be compiled with a larger code model because the linker does not guarantee any specific ordering of data sections from the inputs, so there will be a good chance that other data objects (not only the table) can be out of the 2 GiB range. And, the definition of code models varies between targets, so "small" may be not suitable for some of them even if it's accepted. So to me for now we should revert it. If we really need to override the code model for some targets we should add -mcmodel= options (depending on the target) into the build system instead.
xry111
force-pushed
the
xry111/revert-cmodel
branch
from
8b11072 to
f1f7d06
Compare
Collaborator
|
Hi. Thanks for explanation. Let's stay on the middle-ground: disable BROTLI_MODEL macro for LoongArch and IA-64. I'll prepare PR for that. |
7 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This reverts commit 103b25f.
That breaks compilation on at least LoongArch and IA-64 with latest GCC because LoongArch GCC does not recognize "small" and IA-64 GCC accepts an identifier (instead of string literal).
Per the ChangeLog it seems the change was for "supporting binaries larger than 2 GiB" but I cannot see how it will work reliably. If a binary will be larger than the limit of the default code model, the entire binary (i.e. each relocatable object file that would consist the binary) must be compiled with a larger code model because the linker does not guarantee any specific ordering of data sections from the inputs, so there will be a good chance that other data objects (not only the table) can be out of the 2 GiB range. And, the definition of code models varies between targets, so "small" may be not suitable for some of them even if it's accepted.
So to me for now we should revert it. If we really need to override the code model for some targets we should add -mcmodel= options (depending on the target) into the build system instead.