swjng opened a new pull request, #19567:
URL: https://github.com/apache/tvm/pull/19567

   ## Summary
   
   `tirx.asin`'s LLVM legalize used a 6-term Taylor series for `|x| < 0.5` with 
wrong recurrence coefficients. The ratios in the code (`9/40`, `25/112`, 
`1225/3456`, `3969/28160`) don't match the real asin series (`9/20`, `25/42`, 
`49/72`, `81/110`), so mid-range inputs lose ~1e-3 of precision — over 1000 
float32 ULP. `acos` inherits it via `π/2 − asin(x)`.
   
   ```
   x=0.47  ORT=0.48929077  TVM(old)=0.48820966  err=-1.08e-3
   ```
   
   The Taylor branch was added in #17945 as the initial implementation, with no 
libm fallback. #18582 later patched only `|x| ≥ 0.5` by routing to the libm 
extern, leaving the buggy mid-range in place. I see no evidence the inline 
series was an intentional fast-path.
   
   ## Fix
   
   Drop the inline series, route the whole domain through the existing 
`asinf`/`acosf` extern, keep the out-of-range NaN guard. Max error over `x ∈ 
[-1, 1]` drops to **2.4e-7** (ULP-grade).
   
   ## Tests
   
   - Re-enable `Asin`/`Acos` in `test_unary` (they were commented out with a 
TODO about Taylor precision loss).
   - Existing `test_asin_acos_boundary_values` (#18582) still passes.
   
   If the inline polynomial was intentional for some target/path, please flag 
it — I'll restore it with corrected coefficients instead.
   
   `Atan` is still disabled; that's a separate `x² + 1` overflow bug (#19560).
   
   Fixes #19563.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to