[flang] Implement sinpi

[c8ef]

No description provided.

[c8ef]

• [flang] Implement tanpi #149527
• [flang] Implement sinpi #149525 👈 (View in Graphite)
• main

This stack of pull requests is managed by Graphite. Learn more about stacking.

[llvmbot]

@llvm/pr-subscribers-flang-semantics

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

Author: Connector Switch (c8ef)

Changes

---

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

4 Files Affected:

• (modified) flang/include/flang/Optimizer/Builder/IntrinsicCall.h (+1)
• (modified) flang/lib/Evaluate/intrinsics.cpp (+1)
• (modified) flang/lib/Optimizer/Builder/IntrinsicCall.cpp (+16)
• (added) flang/test/Lower/Intrinsics/sinpi.f90 (+22)

diff --git a/flang/include/flang/Optimizer/Builder/IntrinsicCall.h b/flang/include/flang/Optimizer/Builder/IntrinsicCall.h
index acdba7c49e6b3..d84d3593ebca6 100644
--- a/flang/include/flang/Optimizer/Builder/IntrinsicCall.h
+++ b/flang/include/flang/Optimizer/Builder/IntrinsicCall.h
@@ -419,6 +419,7 @@ struct IntrinsicLibrary {
   mlir::Value genShiftA(mlir::Type resultType, llvm::ArrayRef<mlir::Value>);
   mlir::Value genSign(mlir::Type, llvm::ArrayRef<mlir::Value>);
   mlir::Value genSind(mlir::Type, llvm::ArrayRef<mlir::Value>);
+  mlir::Value genSinpi(mlir::Type, llvm::ArrayRef<mlir::Value>);
   fir::ExtendedValue genSize(mlir::Type, llvm::ArrayRef<fir::ExtendedValue>);
   fir::ExtendedValue genSizeOf(mlir::Type, llvm::ArrayRef<fir::ExtendedValue>);
   mlir::Value genSpacing(mlir::Type resultType,
diff --git a/flang/lib/Evaluate/intrinsics.cpp b/flang/lib/Evaluate/intrinsics.cpp
index 9957010684d48..d44239b41fa20 100644
--- a/flang/lib/Evaluate/intrinsics.cpp
+++ b/flang/lib/Evaluate/intrinsics.cpp
@@ -957,6 +957,7 @@ static const IntrinsicInterface genericIntrinsicFunction[]{
     {"sin", {{"x", SameFloating}}, SameFloating},
     {"sind", {{"x", SameFloating}}, SameFloating},
     {"sinh", {{"x", SameFloating}}, SameFloating},
+    {"sinpi", {{"x", SameFloating}}, SameFloating},
     {"size",
         {{"array", AnyData, Rank::arrayOrAssumedRank},
             OptionalDIM, // unless array is assumed-size
diff --git a/flang/lib/Optimizer/Builder/IntrinsicCall.cpp b/flang/lib/Optimizer/Builder/IntrinsicCall.cpp
index d77a656158a37..823b1eb887992 100644
--- a/flang/lib/Optimizer/Builder/IntrinsicCall.cpp
+++ b/flang/lib/Optimizer/Builder/IntrinsicCall.cpp
@@ -901,6 +901,7 @@ static constexpr IntrinsicHandler handlers[]{
      {{{"number", asValue}, {"handler", asAddr}, {"status", asAddr}}},
      /*isElemental=*/false},
     {"sind", &I::genSind},
+    {"sinpi", &I::genSinpi},
     {"size",
      &I::genSize,
      {{{"array", asBox},
@@ -8060,6 +8061,21 @@ mlir::Value IntrinsicLibrary::genSind(mlir::Type resultType,
   return getRuntimeCallGenerator("sin", ftype)(builder, loc, {arg});
 }
 
+// SINPI
+mlir::Value IntrinsicLibrary::genSinpi(mlir::Type resultType,
+                                       llvm::ArrayRef<mlir::Value> args) {
+  assert(args.size() == 1);
+  mlir::MLIRContext *context = builder.getContext();
+  mlir::FunctionType ftype =
+      mlir::FunctionType::get(context, {resultType}, {args[0].getType()});
+  llvm::APFloat pi = llvm::APFloat(llvm::numbers::pi);
+  mlir::Value dfactor =
+      builder.createRealConstant(loc, mlir::Float64Type::get(context), 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("sin", ftype)(builder, loc, {arg});
+}
+
 // SIZE
 fir::ExtendedValue
 IntrinsicLibrary::genSize(mlir::Type resultType,
diff --git a/flang/test/Lower/Intrinsics/sinpi.f90 b/flang/test/Lower/Intrinsics/sinpi.f90
new file mode 100644
index 0000000000000..38c2277892ec7
--- /dev/null
+++ b/flang/test/Lower/Intrinsics/sinpi.f90
@@ -0,0 +1,22 @@
+! RUN: %flang_fc1 -emit-fir %s -o - | FileCheck %s --check-prefixes="CHECK"
+
+function test_real4(x)
+  real :: x, test_real4
+  test_real4 = sinpi(x)
+end function
+
+! CHECK-LABEL: @_QPtest_real4
+! CHECK: %[[dfactor:.*]] = arith.constant 3.1415926535897931 : f64
+! CHECK: %[[factor:.*]] = fir.convert %[[dfactor]] : (f64) -> f32
+! CHECK: %[[mul:.*]] = arith.mulf %{{.*}}, %[[factor]] fastmath<contract> : f32
+! CHECK: %[[sin:.*]] = math.sin %[[mul]] fastmath<contract> : f32
+
+function test_real8(x)
+  real(8) :: x, test_real8
+  test_real8 = sinpi(x)
+end function
+
+! CHECK-LABEL: @_QPtest_real8
+! CHECK: %[[dfactor:.*]] = arith.constant 3.1415926535897931 : f64
+! CHECK: %[[mul:.*]] = arith.mulf %{{.*}}, %[[dfactor]] fastmath<contract> : f64
+! CHECK: %[[sin:.*]] = math.sin %[[mul]] fastmath<contract> : f64

[tblah]

Why not create the real constant with the argument type in the first place? This might matter for real(16).

[c8ef]

mlir::Value factor = builder.createRealConstant(loc, args[0].getType(), pi);

Using the argument type in the creation process seems to result in the following error:

error: FloatAttr type doesn't match the type implied by its value
flang: /home/c8ef/llvm-dev/mlir/include/mlir/IR/StorageUniquerSupport.h:180: static ConcreteT mlir::detail::StorageUserBase<mlir::FloatAttr, mlir::Attribute, mlir::detail::FloatAttrStorage, mlir::detail::AttributeUniquer, mlir::TypedAttr::Trait>::get(MLIRContext *, Args &&...) [ConcreteT = mlir::FloatAttr, BaseT = mlir::Attribute, StorageT = mlir::detail::FloatAttrStorage, UniquerT = mlir::detail::AttributeUniquer, Traits = <mlir::TypedAttr::Trait>, Args = <mlir::Type &, const llvm::APFloat &>]: Assertion `succeeded( ConcreteT::verifyInvariants(getDefaultDiagnosticEmitFn(ctx), args...))' failed.
PLEASE submit a bug report to https://github.com/llvm/llvm-project/issues/ and include the crash backtrace.
Stack dump:
0.	Program arguments: /home/c8ef/llvm-dev/build/bin/flang -fc1 -emit-fir /home/c8ef/llvm-dev/flang/test/Lower/Intrinsics/sinpi.f90 -o -
 #0 0x0000ffffa6de9e50 llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) /home/c8ef/llvm-dev/llvm/lib/Support/Unix/Signals.inc:834:11
 #1 0x0000ffffa6dea368 PrintStackTraceSignalHandler(void*) /home/c8ef/llvm-dev/llvm/lib/Support/Unix/Signals.inc:918:1
 #2 0x0000ffffa6de83f4 llvm::sys::RunSignalHandlers() /home/c8ef/llvm-dev/llvm/lib/Support/Signals.cpp:104:5
 #3 0x0000ffffa6deab18 SignalHandler(int, siginfo_t*, void*) /home/c8ef/llvm-dev/llvm/lib/Support/Unix/Signals.inc:426:38
 #4 0x0000ffffaa031808 (linux-vdso.so.1+0x808)
 #5 0x0000ffffa64d7dc0 __pthread_kill_implementation ./nptl/pthread_kill.c:44:76
 #6 0x0000ffffa6486980 raise ./signal/../sysdeps/posix/raise.c:27:6
 #7 0x0000ffffa6471ac4 abort ./stdlib/abort.c:81:3
 #8 0x0000ffffa647f9bc __assert_fail_base ./assert/assert.c:53:15
 #9 0x0000ffff9fc572cc mlir::FloatAttr mlir::detail::StorageUserBase<mlir::FloatAttr, mlir::Attribute, mlir::detail::FloatAttrStorage, mlir::detail::AttributeUniquer, mlir::TypedAttr::Trait>::get<mlir::Type&, llvm::APFloat const&>(mlir::MLIRContext*, mlir::Type&, llvm::APFloat const&) /home/c8ef/llvm-dev/mlir/include/mlir/IR/StorageUniquerSupport.h:179:5
#10 0x0000ffff9fc4c8c8 mlir::FloatAttr::get(mlir::Type, llvm::APFloat const&) /home/c8ef/llvm-dev/build/tools/mlir/include/mlir/IR/BuiltinAttributes.cpp.inc:293:10
#11 0x0000ffff9fc3b3bc mlir::Builder::getFloatAttr(mlir::Type, llvm::APFloat const&) /home/c8ef/llvm-dev/mlir/lib/IR/Builders.cpp:254:10
#12 0x0000ffff944e0b84 fir::FirOpBuilder::createRealConstant(mlir::Location, mlir::Type, llvm::APFloat const&) /home/c8ef/llvm-dev/flang/lib/Optimizer/Builder/FIRBuilder.cpp:187:17
#13 0x0000ffff94567eac fir::IntrinsicLibrary::genSinpi(mlir::Type, llvm::ArrayRef<mlir::Value>) /home/c8ef/llvm-dev/flang/lib/Optimizer/Builder/IntrinsicCall.cpp:8072:32

[c8ef]

LogicalResult FloatAttr::verify(function_ref<InFlightDiagnostic()> emitError,
                                Type type, APFloat value) {
  // Verify that the type is correct.
  if (!llvm::isa<FloatType>(type))
    return emitError() << "expected floating point type";

  // Verify that the type semantics match that of the value.
  if (&llvm::cast<FloatType>(type).getFloatSemantics() !=
      &value.getSemantics()) {
    return emitError()
           << "FloatAttr type doesn't match the type implied by its value";
  }
  return success();
}

It appears these two floating-point types have different semantics. To ensure pi uses the same semantics as the argument type, we'd still need to convert it from APFloat. Either way, a conversion seems necessary.

[tblah]

Ahh okay what I think is happening here is that APFloat is forcing its size to f64 because llvm::numbers::pi is defined as a double. I'm concerned that this could produce inaccurate results for real(16) (aka floating point types with more precision than f64: f128 on aarch64 systems, I think f80 on x86).

[c8ef]

Yeah so I think the ultimate solution for trig-pi functions is to use the glibc version, when we have glibc>=2.41(it does provide f128 version). And if not maybe we can use this as a fallback or something like libquadmath. Or maybe we need a interface return pi based on fltSemantic. Not quite sure how to do it right since there are a lot of them.