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

[Thomas Köppe via Phabricator via llvm-commits]

tkoeppe added inline comments.


================
Comment at: llvm/tools/llc/llc.cpp:599
       Target->setCodeModel(*CM_IR);
+    if (auto LDT = codegen::getExplicitLargeDataThreshold())
+      Target->setLargeDataThreshold(*LDT);
----------------
MaskRay wrote:
> aeubanks wrote:
> > tkoeppe wrote:
> > > Can we avoid the implicit check for 0 here and spell out `; LDT > 0`? Does LLVM require C++17?
> > yeah LLVM uses C++17
> > 
> > `codegen::getExplicitLargeDataThreshold` returns a `std::optional`, so we're checking if the user explicitly overrode the value, not if the value is 0. we would want to set the value to 0 if the user explicitly requested it
> Building llvm-project requires c++17 (libomptarget may be an exception).
> 
> We can use `if (auto LDT = ...; LDT && LDT > 0)` only if the default is 0.
Ah yes, sorry, I forgot that it's an optional<int>, not just an int. In that case all I meant was for the check to not be implicit, i.e. I'd recommend:

    for (auto LDT = ...; LDT.has_vaiue()) {
      // use *LDT
    }

Just so there's one less implicit thing one needs to keep in mind when reading this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149288