Skip to content

[Flang] Fix ACOSD and ASIND (fixes issue #145593) #145656

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

mjklemm
Copy link
Contributor

@mjklemm mjklemm commented Jun 25, 2025

Original implementation converted DEG->RAD before calling ACOS/ASIN, but the conversion needs to happen after invoking ACOS/ASIN.

Original implementation converted DEG->RAD before calling ACOS/ASIN, but
the conversion needs to happen after invoking ACOS/ASIN.
@mjklemm mjklemm requested a review from Copilot June 25, 2025 09:14
@mjklemm mjklemm self-assigned this Jun 25, 2025
@mjklemm mjklemm added the bug Indicates an unexpected problem or unintended behavior label Jun 25, 2025
@llvmbot llvmbot added flang Flang issues not falling into any other category flang:fir-hlfir labels Jun 25, 2025
@llvmbot
Copy link
Member

llvmbot commented Jun 25, 2025

@llvm/pr-subscribers-flang-fir-hlfir

Author: Michael Klemm (mjklemm)

Changes

Original implementation converted DEG->RAD before calling ACOS/ASIN, but the conversion needs to happen after invoking ACOS/ASIN.


Full diff: https://github.com/llvm/llvm-project/pull/145656.diff

1 Files Affected:

  • (modified) flang/lib/Optimizer/Builder/IntrinsicCall.cpp (+8-6)
diff --git a/flang/lib/Optimizer/Builder/IntrinsicCall.cpp b/flang/lib/Optimizer/Builder/IntrinsicCall.cpp
index 178b6770d6b53..9700b765ea6f3 100644
--- a/flang/lib/Optimizer/Builder/IntrinsicCall.cpp
+++ b/flang/lib/Optimizer/Builder/IntrinsicCall.cpp
@@ -2645,16 +2645,17 @@ mlir::Value IntrinsicLibrary::genAbs(mlir::Type resultType,
 // ACOSD
 mlir::Value IntrinsicLibrary::genAcosd(mlir::Type resultType,
                                        llvm::ArrayRef<mlir::Value> args) {
+  // maps ACOSD to ACOS * 180 / pi
   assert(args.size() == 1);
   mlir::MLIRContext *context = builder.getContext();
   mlir::FunctionType ftype =
       mlir::FunctionType::get(context, {resultType}, {args[0].getType()});
+  mlir::Value result = getRuntimeCallGenerator("acos", ftype)(builder, loc, {args[0]});
   llvm::APFloat pi = llvm::APFloat(llvm::numbers::pi);
   mlir::Value dfactor = builder.createRealConstant(
-      loc, mlir::Float64Type::get(context), pi / llvm::APFloat(180.0));
+      loc, mlir::Float64Type::get(context), llvm::APFloat(180.0) / pi);
   mlir::Value factor = builder.createConvert(loc, args[0].getType(), dfactor);
-  mlir::Value arg = builder.create<mlir::arith::MulFOp>(loc, args[0], factor);
-  return getRuntimeCallGenerator("acos", ftype)(builder, loc, {arg});
+  return builder.create<mlir::arith::MulFOp>(loc, result, factor);
 }
 
 // ADJUSTL & ADJUSTR
@@ -2796,16 +2797,17 @@ IntrinsicLibrary::genAny(mlir::Type resultType,
 // ASIND
 mlir::Value IntrinsicLibrary::genAsind(mlir::Type resultType,
                                        llvm::ArrayRef<mlir::Value> args) {
+  // maps ASIND to ASIN * 180 / pi
   assert(args.size() == 1);
   mlir::MLIRContext *context = builder.getContext();
   mlir::FunctionType ftype =
       mlir::FunctionType::get(context, {resultType}, {args[0].getType()});
+  mlir::Value result = getRuntimeCallGenerator("asin", ftype)(builder, loc, {args[0]});
   llvm::APFloat pi = llvm::APFloat(llvm::numbers::pi);
   mlir::Value dfactor = builder.createRealConstant(
-      loc, mlir::Float64Type::get(context), pi / llvm::APFloat(180.0));
+      loc, mlir::Float64Type::get(context), llvm::APFloat(180.0) / pi);
   mlir::Value factor = builder.createConvert(loc, args[0].getType(), dfactor);
-  mlir::Value arg = builder.create<mlir::arith::MulFOp>(loc, args[0], factor);
-  return getRuntimeCallGenerator("asin", ftype)(builder, loc, {arg});
+  return builder.create<mlir::arith::MulFOp>(loc, result, factor);
 }
 
 // ATAND, ATAN2D

Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR fixes the conversion bug for ACOSD and ASIND by moving the degree conversion calculation to after the runtime calls of ACOS and ASIN, ensuring the correct order of operations.

  • Update ACOSD to compute the degree conversion multiplier (180.0 / pi) and multiply the acos call’s result.
  • Update ASIND similarly to compute and apply the correct conversion factor.

Copy link

github-actions bot commented Jun 25, 2025

✅ With the latest revision this PR passed the C/C++ code formatter.

@klausler klausler removed their request for review June 25, 2025 15:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Indicates an unexpected problem or unintended behavior flang:fir-hlfir flang Flang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants