[PATCH] D84877: Support for soft fp16 to fp64 IEEE conversions

[Luke Drummond via Phabricator via llvm-commits]

ldrumm added inline comments.


================
Comment at: compiler-rt/test/builtins/Unit/extendhfdf2_test.c:9
+
+double __extendhfdf2(uint16_t a);
+
----------------
atrosinenko wrote:
> Shouldn't it be marked with `COMPILER_RT_ABI` identically to the corresponding definition?
Yes you're right. It seems most of the other tests do this now. I'll update


================
Comment at: compiler-rt/test/builtins/Unit/extendhfdf2_test.c:11
+
+char assumption_1[sizeof(__fp16) * CHAR_BIT == 16] = {0};
+
----------------
atrosinenko wrote:
> There is a `COMPILE_TIME_ASSERT(expr)` macro inside the `int_util.h`. Grepping the `assumption` string shows lots of relevant lines under `compiler-rt/test/builtins/Unit`. On the other hand, `COMPILE_TIME_ASSERT` is used sometimes as well. I wonder whether it is against the coding style **for tests** to use too generic macro. Or maybe those manually defined `assumption_N`s just predates introduction of that descriptive macro.
Good suggestion; will fix


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84877