[PATCH] D43106: [RISCV] Enable -fuse-int128 through cmake flag COMPILER_RT_HAS_FINT128_FLAG

[Eli Friedman via Phabricator via llvm-commits]

efriedma added reviewers: compnerd, howard.hinnant.
efriedma added inline comments.


================
Comment at: lib/builtins/CMakeLists.txt:505
   append_list_if(COMPILER_RT_HAS_STD_C11_FLAG -std=c11 BUILTIN_CFLAGS)
+  append_list_if(COMPILER_RT_HAS_FINT128_FLAG -fuse-int128 BUILTIN_CFLAGS)
 
----------------
efriedma wrote:
> kito-cheng wrote:
> > kito-cheng wrote:
> > > Does it possible only append this flag for target which has sizeof (long double) == 16 and !defined(__SIZEOF_INT128__)?
> > Sorry I forgot to escape the code : `defined(__SIZEOF_FLOAT128__) && !defined(__SIZEOF_INT128__)`
> I think it makes sense to always to force this, even if sizeof(long double) isn't 16; the 128-bit integer functions could be useful for other users.
Actually, thinking about it a bit more, I'm not sure it's a good idea to turn this on automatically for every target.

The key issue here is that for targets where __int128_t isn't legal, it doesn't have a defined ABI.  clang will automatically come up with some ABI rules if you pass -fforce-enable-int128, but it might not be the same ABI that the author of a revision of the ABI document would choose.   So if a future revision of the ABI does choose some ABI rules, we're stuck with a bunch of compiler-rt routines using the wrong rules.

For 32-bit RISCV specifically, we can avoid this risk by defining rules for int128_t in the ABI document now, even if we expect compilers won't support it by default.  Not sure for other targets.


Repository:
  rCRT Compiler Runtime

https://reviews.llvm.org/D43106