[clang] [llvm] [clang] Reland Add tanf16 builtin and support for tan constrained intrinsic (PR #94559)

Farzon Lotfi via cfe-commits cfe-commits at lists.llvm.org
Wed Jun 12 20:15:38 PDT 2024


farzonl wrote:

It seems like we have four options here. We can drop  the `def Tan : FPMathTemplate, LibBuiltin<"math.h">` builtins so no  `Builtin::BItanf` or `Builtin::BItanl` in cgbuitlin switch case. That wouldn't solve the SLPVectorizer  case  but doesn't expose it either unless you use a clang_builtin function which should limit the impact. The tan library functions would behave as before.

Option 2 we land an arm7 backend for Tan.  That could work for you, but the issue you raised with the `SLPVectorizer` case likely means this same issue exists for powerPC, RISCV, and potentially other backends.

Option 3.  we guard the  emitter with a target check for wasm (maybe), x86, and aarch64
```cpp
    case Builtin::BItanf:
    case Builtin::BItanl:
    case Builtin::BI__builtin_tan:
    case Builtin::BI__builtin_tanf:
    case Builtin::BI__builtin_tanf16:
    case Builtin::BI__builtin_tanl:
    case Builtin::BI__builtin_tanf128: {
      switch(CGF.getTarget().getTriple().getArch()) {
           case llvm::Triple::aarch64:
           case llvm::Triple::x86:
           case llvm::Triple::x86_64:
           case llvm::Triple::wasm32:
           case llvm::Triple::wasm64:
               return RValue::get(emitUnaryMaybeConstrainedFPBuiltin(
                      *this, E, Intrinsic::tan, Intrinsic::experimental_constrained_tan));
      }
}
```

Option 4 we revert this change, The test cases would go stale and it also means our plans for landing constraint intrinsics needs to go in the backlog, because it means constraint intrinsics don't make sense until tan has full support across all backends.

https://github.com/llvm/llvm-project/pull/94559


More information about the cfe-commits mailing list