[PATCH] D112401: [Clang] Mutate printf bulitin names under IEEE128 on PPC64

[Qiu Chaofan via Phabricator via cfe-commits]

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