[PATCH] D52023: [ThinLTO]Allow setting of maximum cache size with 64-bit number, and provide C-interface function for large values
James Henderson via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Thu Sep 13 06:37:17 PDT 2018
jhenderson added inline comments.
================
Comment at: include/llvm-c/lto.h:828
extern void thinlto_codegen_set_cache_size_bytes(thinlto_code_gen_t cg,
unsigned max_size_bytes);
----------------
tejohnson wrote:
> Shouldn't this be changed to uint64_t? I don't think that would break any existing users since it is wider.
That's what I originally thought (and I would prefer to do it if possible) but I think there are two issues, which may both be irrelevant:
1) Is uint64_t definitely available in all versions of C that this header is intended to be compatible with? I don't know what the compatibility guarantees are.
2) In the past, my team have occasionally updated the LTO library (as a DLL) without updating the header in our linker's source tree. If somebody were to do this in the future, (from one with the old, smaller size, to one with the newer size), I think this would result in unexpected behaviour (because the caller thinks it's calling an unsigned version, but the function called actually takes a uint64_t). In our case, that won't happen, since I'll be updating our local header once we integrate these changes in our downstream port, but I wasn't sure if this is something that might affect other users. I couldn't find any documentation explaining the compatibility guarantees of the C interface.
Repository:
rL LLVM
https://reviews.llvm.org/D52023
More information about the llvm-commits
mailing list