[flang] Implement tand intrinsic

[DavidTruby]

Summary

This implements the tand intrinsic by performing a multiplication
by pi/180 to the argument before calling tan inline.

This is a commonly provided extension that is used by OpenRadioss

[kiranchandramohan]

Could you add this to flang/docs/Extensions.md?

flang/lib/Optimizer/Builder/IntrinsicCall.cpp

4996

Can you add a TODO for the complex type?

[vzakhari]

Thank you for the changes! It looks good to me.

Will you also support constant folding for tand (in flang/lib/Evaluate/fold-real.cpp)? If not, please put a note about that into flang/docs/Extensions.md.

[mmuetzel]

If I understand correctly, one of the (more important?) use cases for tand is that it returns exactly 0 for large multiples of 180. In contrast tan might return a floating point number that might slightly deviate from 0 for large multiples of pi.

Is the proposed implementation "more exact" for large multiples of 180 (compared to large multiples of pi with tan)?

[DavidTruby]

In D154614#4486193, @mmuetzel wrote:

If I understand correctly, one of the (more important?) use cases for tand is that it returns exactly 0 for large multiples of 180. In contrast tan might return a floating point number that might slightly deviate from 0 for large multiples of pi.

Is the proposed implementation "more exact" for large multiples of 180 (compared to large multiples of pi with tan)?

This implementation won't do that no, although the implementation in classic flang and gfortran doesn't appear to do that either. Classic flang and gfortran do both provide for tand(90) being Inf, which it won't be with this implementation though. I'm not really aware of how important this is, or how other compilers do it. I suppose we could have a runtime function that checks for both these cases?

[DavidTruby]

For what it's worth, tand is in the draft Fortran 2023 standard, in which no guarantees are given about these edge cases (in fact, it explicitly uses tand(180) being approximately 0 as an example). I'm open to adding the edge cases if those are necessary for applications though; this patch will need reworking to call into a runtime library function instead of doing inline MLIR so there would be a performance tradeoff.

[mmuetzel]

Would it make sense to do the reduction to the interval from -90 to +90 before the conversion from degrees to radians?
That reduction would most likely be more accurate for integers (most specific multiples of 180) than what can be done in finite precision after the conversion (i.e., by the tan function).

[DavidTruby]

I think we can assume that reduction in range will be being done by the implementation of tan we call anyway, would there be an advantage to doing it again outside?

I guess my feeling is this: the standard doesn't provide for any kind of extra accuracy to special values in this function, and based on other compilers it doesn't seem like the user can assume extra accuracy across different compilers. We also don't have any kind of special information inside the runtime that the caller doesn't have anyway, so the user could check these special cases themself and will have to if they want portability.

As a result I say we should just do the fastest thing, and let users that need the extra accuracy check for it outside the intrinsic call. This will avoid the special case happening for users that don't need it, and avoid it happening twice for those that do.

I don't feel particularly strongly about this though, if others think we do need extra checks I'm happy to update the patch with them.

[DavidTruby]

In D154614#4478134, @vzakhari wrote:

Thank you for the changes! It looks good to me.

Will you also support constant folding for tand (in flang/lib/Evaluate/fold-real.cpp)? If not, please put a note about that into flang/docs/Extensions.md.

I think I will do this in a separate patch, if that's ok? I can put a todo in this patch meanwhile

[kiranchandramohan]

LG.

I think @mmuetzel probably makes a good point from a user perspective. I tried to look into this in a bit more detail but could not find details about this motivation.

The early j3 papers (https://j3-fortran.org/doc/year/18/18-139.txt, https://j3-fortran.org/doc/year/18/18-272.txt) do not mention exactly 0 for large multiples of 180 as a requirement or motivation. The motivation seems to be that this is implemented in multiple compilers and hence should be standardized.

The implementations in compilers seems to broadly follow what is being done here.
https://gcc.gnu.org/onlinedocs/gfortran/Extended-math-intrinsics.html

For advanced users, it may be important to know the implementation of these functions. They are simply wrappers around the standard radian functions, which have more accurate builtin versions. These functions convert their arguments (or results) to degrees (or radians) by taking the value modulus 360 (or 2*pi) and then multiplying it by a constant radian-to-degree (or degree-to-radian) factor, as appropriate. The factor is computed at compile-time as 180/pi (or pi/180).

https://docs.oracle.com/cd/E19205-01/820-4180/man3m/trig_sun.3m.html

sind(x), cosd(x), and tand(x) return trigonometric functions
of   degree   arguments.    sind(x):=   sin(x*pi/180).

Note: The j3 proposals page for this is https://github.com/j3-fortran/fortran_proposals/issues/164.

Please wait a day before submitting incase there are different opinions.

[DavidTruby]

I have pushed this as-is, and we can add some extra precision afterwards if we think it's necessary. Hopefully that's ok!

[klausler]

In D154614#4514617, @DavidTruby wrote:

I have pushed this as-is, and we can add some extra precision afterwards if we think it's necessary. Hopefully that's ok!

The degree and half-cycle trig functions like TAND that are becoming standard in Fortran 2023 should not be implemented like this (multiplication of arguments or results). Libraries with accurate implementations should be used instead.

[tschuett]

Could you introduce a TandOp for optimizations?

flang/include/flang/Optimizer/Builder/IntrinsicCall.h

struct IntrinsicLibrary {
   fir::ExtendedValue genSize(mlir::Type, llvm::ArrayRef<fir::ExtendedValue>);
   mlir::Value genSpacing(mlir::Type resultType,
                          llvm::ArrayRef<mlir::Value> args);
   fir::ExtendedValue genSpread(mlir::Type, llvm::ArrayRef<fir::ExtendedValue>);
   fir::ExtendedValue genStorageSize(mlir::Type,
                                     llvm::ArrayRef<fir::ExtendedValue>);
   fir::ExtendedValue genSum(mlir::Type, llvm::ArrayRef<fir::ExtendedValue>);
   void genSystemClock(llvm::ArrayRef<fir::ExtendedValue>);
+  mlir::Value genTand(mlir::Type, llvm::ArrayRef<mlir::Value>);
   mlir::Value genTrailz(mlir::Type, llvm::ArrayRef<mlir::Value>);
   fir::ExtendedValue genTransfer(mlir::Type,
                                  llvm::ArrayRef<fir::ExtendedValue>);
   fir::ExtendedValue genTranspose(mlir::Type,
                                   llvm::ArrayRef<fir::ExtendedValue>);
   fir::ExtendedValue genTrim(mlir::Type, llvm::ArrayRef<fir::ExtendedValue>);
   fir::ExtendedValue genUbound(mlir::Type, llvm::ArrayRef<fir::ExtendedValue>);
   fir::ExtendedValue genUnpack(mlir::Type, llvm::ArrayRef<fir::ExtendedValue>);

flang/lib/Optimizer/Builder/IntrinsicCall.cpp

static constexpr IntrinsicHandler handlers[]{
      {{{"array", asBox},
        {"dim", asValue},
        {"mask", asBox, handleDynamicOptional}}},
      /*isElemental=*/false},
     {"system_clock",
      &I::genSystemClock,
      {{{"count", asAddr}, {"count_rate", asAddr}, {"count_max", asAddr}}},
      /*isElemental=*/false},
+    {"tand", &I::genTand},
     {"trailz", &I::genTrailz},
     {"transfer",
      &I::genTransfer,
      {{{"source", asAddr}, {"mold", asAddr}, {"size", asValue}}},
      /*isElemental=*/false},
     {"transpose",
      &I::genTranspose,
      {{{"matrix", asAddr}}},
return builder
         mlir::Value size = builder.createConvert(
             loc, resultType,
             fir::runtime::genSizeDim(builder, loc, array, dimValue));
         builder.create<fir::ResultOp>(loc, size);
       })
       .getResults()[0];
 }
 
+// TAND
+mlir::Value IntrinsicLibrary::genTand(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::FloatType::getF64(context), pi / llvm::APFloat(180.0));
+  mlir::Value factor = builder.createConvert(loc, args[0].getType(), dfactor);
+  mlir::Value arg = builder.create<mlir::arith::MulFOp>(loc, args[0], factor);
+  return getRuntimeCallGenerator("tan", ftype)(builder, loc, {arg});
+}
+
 // TRAILZ
 mlir::Value IntrinsicLibrary::genTrailz(mlir::Type resultType,
                                         llvm::ArrayRef<mlir::Value> args) {
   assert(args.size() == 1);
 
   mlir::Value result =
       builder.create<mlir::math::CountTrailingZerosOp>(loc, args);
 

flang/test/Lower/Intrinsics/tand.f90

+! RUN: bbc -emit-fir %s -o - | FileCheck %s --check-prefixes="CHECK,CHECK-FAST"
+! RUN: bbc --math-runtime=precise -emit-fir %s -o - | FileCheck %s --check-prefixes="CHECK,CHECK-PRECISE"
+! RUN: %flang_fc1 -emit-fir %s -o - | FileCheck %s --check-prefixes="CHECK,CHECK-FAST"
+
+function test_real4(x)
+  real :: x, test_real4
+  test_real4 = tand(x)
+end function
+
+! CHECK-LABEL: @_QPtest_real4
+! CHECK: %[[dfactor:.*]] = arith.constant 0.017453292519943295 : f64
+! CHECK: %[[factor:.*]] = fir.convert %[[dfactor]] : (f64) -> f32
+! CHECK: %[[arg:.*]] = arith.mulf %{{[A-Za-z0-9._]+}}, %[[factor]] fastmath<contract> : f32
+! CHECK-PRECISE: %{{.*}} = fir.call @tanf(%[[arg]]) fastmath<contract> : (f32) -> f32
+! CHECK-FAST: %{{.*}} = math.tan %[[arg]] fastmath<contract> : f32
+
+function test_real8(x)
+  real(8) :: x, test_real8
+  test_real8 = tand(x)
+end function
+
+! CHECK-LABEL: @_QPtest_real8
+! CHECK: %[[factor:.*]] = arith.constant 0.017453292519943295 : f64
+! CHECK: %[[arg:.*]] = arith.mulf %{{[A-Za-z0-9._]+}}, %[[factor]] fastmath<contract> : f64
+! CHECK-PRECISE: %{{.*}} = fir.call @tan(%[[arg]]) fastmath<contract> : (f64) -> f64
+! CHECK-FAST: %{{.*}} = math.tan %[[arg]] fastmath<contract> : f64