[PATCH] D85910: libclc: Add a __builtin to let SPIRV targets select between SW and HW FMA

Jesse Natalie via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sat Aug 22 09:01:32 PDT 2020


jenatali requested changes to this revision.
jenatali added inline comments.
This revision now requires changes to proceed.


================
Comment at: libclc/generic/lib/math/math.h:44
+#elif defined CLC_SPIRV || defined CLC_SPIRV64
+bool __builtin_has_hw_fma32(void);
+#define HAVE_HW_FMA32() __builtin_has_hw_fma32()
----------------
>From discussion on the corresponding Mesa merge request that would handle this builtin function call (https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/6035#note_606348), I think this would be better as defined, non-inlined function. That way, we get the best of both worlds:

  - If Mesa understands the builtin, it can choose to override it with its own implementation, rather than using the one provided by libclc, allowing individual drivers to control the behavior.
  - If mesa doesn't understand the builtin, then it's just a regular function call, and after non-LLVM optimizations, it will eventually end up inlined, and the dead code for the non-taken path can be removed.

We might need to have a dedicated .cl file for the definition though, rather than defining it inline, just to avoid violating the one-definition-rule here. And we should probably do the same thing for subnormal controls, rather than just setting them to false.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85910



More information about the llvm-commits mailing list