[PR #1368] [CLOSED] Revert "explicitly specify model for relocatable variables" #1975

New Issue

claunia · 2026-01-29T20:56:44Z

claunia commented

2026-01-29 20:56:44 +00:00

📋 Pull Request Information

Original PR: https://github.com/google/brotli/pull/1368
Author: @xry111
Created: 10/29/2025
Status: ❌ Closed

Base: master ← Head: xry111/revert-cmodel

📝 Commits (1)

f1f7d06 Revert "explicitly specify model for relocatable variables"

📊 Changes

30 files changed (+63 additions, -74 deletions)

View changed files

📝 c/common/constants.c (+1 -1)
📝 c/common/constants.h (+1 -1)
📝 c/common/context.c (+1 -1)
📝 c/common/dictionary.c (+2 -2)
📝 c/common/dictionary_inc.h (+1 -1)
📝 c/common/platform.h (+0 -6)
📝 c/common/transform.c (+4 -4)
📝 c/dec/bit_reader.c (+1 -1)
📝 c/dec/decode.c (+3 -3)
📝 c/dec/huffman.c (+1 -1)
📝 c/dec/prefix.c (+0 -1)
📝 c/dec/prefix.h (+2 -2)
📝 c/dec/prefix_inc.h (+1 -1)
📝 c/enc/backward_references_hq.c (+2 -2)
📝 c/enc/brotli_bit_stream.c (+3 -3)
📝 c/enc/command.h (+4 -4)
📝 c/enc/compress_fragment.c (+1 -1)
📝 c/enc/compress_fragment_two_pass.c (+2 -2)
📝 c/enc/dictionary_hash.c (+0 -2)
📝 c/enc/dictionary_hash.h (+4 -4)

...and 10 more files

📄 Description

This reverts commit 103b25fb1e.

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.

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/google/brotli/pull/1368 **Author:** [@xry111](https://github.com/xry111) **Created:** 10/29/2025 **Status:** ❌ Closed **Base:** `master` ← **Head:** `xry111/revert-cmodel` --- ### 📝 Commits (1) - [`f1f7d06`](https://github.com/google/brotli/commit/f1f7d0651bb810526263a8ff6517dfab0168312d) Revert "explicitly specify model for relocatable variables" ### 📊 Changes **30 files changed** (+63 additions, -74 deletions) <details> <summary>View changed files</summary> 📝 `c/common/constants.c` (+1 -1) 📝 `c/common/constants.h` (+1 -1) 📝 `c/common/context.c` (+1 -1) 📝 `c/common/dictionary.c` (+2 -2) 📝 `c/common/dictionary_inc.h` (+1 -1) 📝 `c/common/platform.h` (+0 -6) 📝 `c/common/transform.c` (+4 -4) 📝 `c/dec/bit_reader.c` (+1 -1) 📝 `c/dec/decode.c` (+3 -3) 📝 `c/dec/huffman.c` (+1 -1) 📝 `c/dec/prefix.c` (+0 -1) 📝 `c/dec/prefix.h` (+2 -2) 📝 `c/dec/prefix_inc.h` (+1 -1) 📝 `c/enc/backward_references_hq.c` (+2 -2) 📝 `c/enc/brotli_bit_stream.c` (+3 -3) 📝 `c/enc/command.h` (+4 -4) 📝 `c/enc/compress_fragment.c` (+1 -1) 📝 `c/enc/compress_fragment_two_pass.c` (+2 -2) 📝 `c/enc/dictionary_hash.c` (+0 -2) 📝 `c/enc/dictionary_hash.h` (+4 -4) _...and 10 more files_ </details> ### 📄 Description This reverts commit 103b25fb1e38ee765c827ce470c85a4d6541df12. 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. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

claunia added the pull-request label 2026-01-29 20:56:44 +00:00

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: starred/brotli#1975