Implement fixtfsi, fixunstfsi, which perform quad-precision to (sign/unsigned)integer conversion
Details
Diff Detail
- Repository
- rL LLVM
Event Timeline
lib/builtins/fixtfsi.c | ||
---|---|---|
31 ↗ | (On Diff #7681) | Typos, see comments below for return value. |
48 ↗ | (On Diff #7681) | Typo. Doesn't agree with the comment above in that it provides a deterministic value? |
50 ↗ | (On Diff #7681) | Shouldn't this be sign == 1 ? INT_MAX : INT_MIN, otherwise it would return INT_MIN + 1? |
lib/builtins/fixunstfsi.c | ||
26 ↗ | (On Diff #7681) | Code does range check? |
42 ↗ | (On Diff #7681) | Typo in comment, please spell the constant as UINT_MAX too. |
This patch blocks quad math stuff on aarch64 from using compiler-rt instead of gnu libgcc
Can this patch be commited, now? Thanks
lib/builtins/fixtfsi.c | ||
---|---|---|
33 ↗ | (On Diff #9792) | See initial comment, no need for range check. |
Refactor existing fixXfsi functions and use common code for fixtfsi. Drop range checks as they are UB.
Since these functions are implemented for using compiler-rt instead of libgcc, and __fixtfsi returns INT_MAX/INT_MIN when the input is out of range in libgcc, I think range check is still needed to avoid inconsistent results.
Steve -- you wrote the original version. Can you comment on the overflow handling please?
I think it makes sense to clamp out-of-range values. Even though the C language cast is UB, there’s a fairly clear “right” thing to do, and we can do it without excessive performance penalty. Let’s follow libgcc here and saturate.
Saturation logic for _fixunstfsi is also needed but it has been removed in previous revision. (i.e, range check part in fixuntfsi.cpp)
Yes, it's not resonable to implement saturation logic for fixtfsi but not for fixuntfsi.
That's reasonable -- I'm not asking if it can be committed in the current state, but rather if it's ready to commit once this is addressed.
OK, but the following two cases in the test will fail if there's no saturation logic in fixunstfsi. I think it's better to remove them, thanks.
if (test__fixunstfsi(0x1.23456789abcdefp+40, UINT32_C(0xffffffff))) return 1; if (test__fixunstfsi(0x1.23456789abcdefp+256, UINT32_C(0xffffffff))) return 1;
Refactor fix* and fixuns* to use common logic and consistent behavior w.r.t. saturation. XF format is still excluded for now.