qiucf added inline comments.

================
Comment at: clang/lib/CodeGen/CGBuiltin.cpp:100
+  static SmallDenseMap<unsigned, StringRef, 8> F128Builtins{
+      {Builtin::BI__builtin_printf, "__printfieee128"},
+      {Builtin::BI__builtin_vsnprintf, "__vsnprintfieee128"},
----------------
jsji wrote:
> Why only these printf builtins? I think there are full list of similar 
> libcalls in `GLIBC_2.32` and later?
Yes, Glibc redirects all these functions depending on long double semantics, 
and GCC has corresponding builtins for almost all library functions 
(https://gcc.gnu.org/onlinedocs/gcc/Other-Builtins.html).

But currently clang doesn't have all of these builtins (for example, 
`__builtin_scanf`). It seems beyond this patch's scope to make the list 
complete.


================
Comment at: clang/lib/CodeGen/CGBuiltin.cpp:117
+    if (getTriple().isPPC64() &&
+        &getTarget().getLongDoubleFormat() == &llvm::APFloat::IEEEquad() &&
+        F128Builtins.find(BuiltinID) != F128Builtins.end())
----------------
jsji wrote:
> How do we deal with the glibc version? Do we assume that user has glibc newer 
> than GLIBC_2.32?
I notice not all libcall symbols are implemented in compiler-rt, which means we 
don't need to always expect all libcalls emitted are available in current libc?

It's surprising that libc version are not checked in clang/driver/toolchain 
part. The only place checking libc version is in 
[sanitizer](https://github.com/llvm/llvm-project/blob/81441cf44c145e1d90a10c743e0190a44fbf8fcb/compiler-rt/lib/sanitizer_common/sanitizer_linux_libcdep.cpp#L170).
 Maybe we can use that to emit warnings, like D112906.


================
Comment at: clang/lib/CodeGen/CGBuiltin.cpp:119
+        F128Builtins.find(BuiltinID) != F128Builtins.end())
+      Name = F128Builtins[BuiltinID];
+    else
----------------
jsji wrote:
> Do we have to do it here? Can we just update the libcall names in 
> `RuntimeLibcalls.def` or `setLibcallName` similar to others?
I think we cannot fill these calls in backend's runtime libcall list, because 
it contains supplementary routine names for legalizing IR 
instruction/intrinsics. Here the builtins are just converted into normal 
function calls in IR.


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

https://reviews.llvm.org/D112401

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to