The number of iterations was incorrectly determined for DP FP vector types and the tests were insufficient to flag this issue.
Probably the releases 4 and 5 of LLVM should also be fixed.
Paths
| Differential D39507
[AArch64] Fix the number of iterations for the Newton series ClosedPublic Authored by evandro on Nov 1 2017, 1:33 PM.
Details Summary The number of iterations was incorrectly determined for DP FP vector types and the tests were insufficient to flag this issue. Probably the releases 4 and 5 of LLVM should also be fixed.
Diff Detail Event TimelineHerald added subscribers: hiraditya, kristof.beyls, javed.absar, rengolin. · View Herald TranscriptNov 1 2017, 1:33 PM Comment Actions LGTM. This is a change I wanted to make soon myself, but never got around to it yet. It would be great if you could upload a full-context diff and wait a bit with committing, to give other people a chance to raise concerns. This revision is now accepted and ready to land.Nov 2 2017, 2:58 AM Comment Actions Thanks Evandro. Out of curiosity, were there any tests/benchmarks, where this caused failures/wrong results? Comment Actions
Yes, with the series expansion enabled, neither CPU2000 nor CPU2006 would validate. With this fix, expanding sqrt() with the series both validate, but not when expanding division. I suspect that the initial estimate for division is insufficient for DP, failing to converge even with additional iterations. Closed by commit rL317349: [AArch64] Fix the number of iterations for the Newton series (authored by evandro). · Explain WhyNov 3 2017, 11:57 AM This revision was automatically updated to reflect the committed changes.
Revision Contents
Diff 121330 llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
llvm/test/CodeGen/AArch64/recp-fastmath.ll
llvm/test/CodeGen/AArch64/sqrt-fastmath.ll
|