gemini-code-assist[bot] commented on code in PR #18582:
URL: https://github.com/apache/tvm/pull/18582#discussion_r2612849178


##########
src/target/llvm/intrin_rule_llvm.cc:
##########
@@ -178,25 +187,45 @@ TVM_REGISTER_OP("tir.asin")
       PrimExpr term9 = term7 * x2 * make_const(x.dtype(), 1225) / 
make_const(x.dtype(), 3456);
       PrimExpr term11 = term9 * x2 * make_const(x.dtype(), 3969) / 
make_const(x.dtype(), 28160);
       PrimExpr series = term1 + term3 + term5 + term7 + term9 + term11;
+      
+      // System library function for boundary values
+      PrimExpr lib_result = DispatchPureExtern<FloatSuffix>(e);
+      
       /* --- domain limit check --- */
       PrimExpr lower = make_const(x.dtype(), -1.0);
       PrimExpr upper = make_const(x.dtype(), 1.0);
-      PrimExpr out_range = tir::Or(x<lower, x> upper);
+      PrimExpr out_range = tir::Or(x < lower, x > upper);
       // Use a quiet NaN constant
       PrimExpr nan_const = make_const(x.dtype(), 
std::numeric_limits<double>::quiet_NaN());
-      // select: if out of [-1,1] → NaN, else → series
-      return tir::Select(out_range, nan_const, series);
+      
+      // select: if out of [-1,1] → NaN, else if |x| >= threshold → lib, else 
→ series
+      return tir::Select(out_range, nan_const,
+                        tir::Select(use_lib, lib_result, series));
     });
 
 TVM_REGISTER_OP("tir.acos")
     .set_attr<FLegalize>("llvm.FLegalize", [](const PrimExpr& e) -> PrimExpr {
       using tir::make_const;
+      using namespace intrin;
       const tir::CallNode* call = e.as<tir::CallNode>();
       ICHECK(call != nullptr) << "Invalid call node in acos legalization";
       const PrimExpr& x = call->args[0];
+      
+      // Use system library function for values near boundaries where ASIN 
Taylor series
+      // has poor precision, which would cause ACOS errors.
+      PrimExpr threshold = make_const(x.dtype(), 0.9);
+      PrimExpr abs_x = tir::abs(x);
+      PrimExpr use_lib = abs_x >= threshold;
+      
+      // For values away from boundaries, use π/2 - asin(x)
       PrimExpr half_pi = make_const(x.dtype(), M_PI / 2);
       PrimExpr asin_x = asin(x);
-      return half_pi - asin_x;
+      PrimExpr formula_result = half_pi - asin_x;
+      
+      // System library function for boundary values
+      PrimExpr lib_result = DispatchPureExtern<FloatSuffix>(e);
+      
+      return tir::Select(use_lib, lib_result, formula_result);

Review Comment:
   ![high](https://www.gstatic.com/codereviewagent/high-priority.svg)
   
   For consistency with the `asin` implementation and for improved robustness, 
it's better to add an explicit domain check for `acos` to ensure it returns 
`NaN` for inputs outside of `[-1, 1]`, instead of relying on the system 
library's behavior. This makes the code more explicit and safer across 
different backends/platforms.
   
   ```c
         /* --- domain limit check --- */
         PrimExpr lower = make_const(x.dtype(), -1.0);
         PrimExpr upper = make_const(x.dtype(), 1.0);
         PrimExpr out_range = tir::Or(x < lower, x > upper);
         // Use a quiet NaN constant
         PrimExpr nan_const = make_const(x.dtype(), 
std::numeric_limits<double>::quiet_NaN());
   
         return tir::Select(out_range, nan_const,
                            tir::Select(use_lib, lib_result, formula_result));
   ```



-- 
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