[libc-commits] [PATCH] D100811: [libc] Add hardware implementations of fma and fmaf for x86_64 and aarch64.

[Siva Chandra via Phabricator via libc-commits]

sivachandra added a comment.

In D100811#2700415 <https://reviews.llvm.org/D100811#2700415>, @tschuett wrote:

> Stupid question: why assembler instead of builtins:
> https://github.com/gcc-mirror/gcc/blob/master/gcc/testsuite/gcc.target/ia64/builtin-fma-1.c
> I guess there is a tradeoff between control over the process and helping the register allocator.

Builtins can potentially silently call into the libc. I preferred inline assembly to avoid such a possibility. I think using target specific compiler intrinsics is a better option but they don't have convenient ways to convert from intrinsic types like `__m128` to and from `float/double`.



================
Comment at: libc/utils/FPUtil/FMA.h:14
+
+#ifdef __x86_64__
+#include "x86_64/FMA.h"
----------------
lntue wrote:
> Do we also need to check for _ _FMA__ flag?
I agree that not checking on `__FMA__` is not ideal. But, I preferred not to condition on any other thing because it complicates the build system. For example, if `__FMA__` was not defined, then we wouldn't get the hardware `fma` and `fmaf` but only get the integer `fmaf` implementation. But, other places will expect that `fma` and `fmaf` both are available. We can of course teach the build system to check for `__FMA__` and propagate that info suitably to the rest of the libc build. That is a change we can take up separately if really required. I would prefer to see more examples of that pattern before building the appropriate machinery in to the build system.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100811