[PATCH] D149288: [X86] Introduce a large data threshold for the medium code model

[Arthur Eubanks via Phabricator via llvm-commits]

aeubanks added a comment.

In D149288#4343075 <https://reviews.llvm.org/D149288#4343075>, @jyknight wrote:

> `large-data-threshold` is ABI, and needs to have a default value which is the same as GCC, 65535. (And users should be discouraged from changing it.)
>
> In many common cases you can get away with using different values in different object files, because cross-object references are going via PLT/GOT when the compiler doesn't know the definition is in-DSO. But that's definitely not a 100% solution -- e.g. `__attribute__((visibility("hidden")))` or ODR-data defined in a -fPIE object is known to be defined in-DSO, so will use a pc-relative reference.

Yes, changing the default value and adding a clang flag will come in a future patch; this patch preserves the current behavior by default



================
Comment at: llvm/lib/Target/TargetMachine.cpp:50
+    const DataLayout &DL = GV->getParent()->getDataLayout();
+    return DL.getTypeSizeInBits(GV->getValueType()) / 8 >= LargeDataThreshold;
+  }
----------------
jyknight wrote:
> W also must handle unknown-sized objects as large here.
> 
> E.g. `clang -mcmodel=medium -fPIC` on
> ```
> __attribute__((visibility("hidden"))) extern int x[];
> int bar(void) { return x[0]; }
> ```
> must refer to x via GOTOFF not pc-relative, because it _could_ be defined as large.
ah ok, although that's not observable in this patch since we only use this for determining which section to place a symbol in, not how we reference a symbol (that's D150297). I'll make the change here but add tests in the other patch


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D149288/new/

https://reviews.llvm.org/D149288