[PATCH] D144454: Add builtin for llvm set rounding

Wed Mar 1 21:58:03 PST 2023

xiongji90 added a comment.

In D144454#4163658 <https://reviews.llvm.org/D144454#4163658>, @sepavloff wrote:

> C standard function `fesetround` accepts rounding mode in the form of target-specific values like `FE_TONEAREST`. They usually represent corresponding bits in control register, so are different for different targets. `llvm.set_rounding` in contrast accepts rounding mode as target-independent value, the encoding is same as used in `FLT_ROUNDS`.
>
> The difference between `fesetround` and `llvm.set_rounding` is same as for `fegetround` and `FLT_ROUNDS`. They differ in how rounding mode is specified, - as target-specific or as target-independent values respectively.
>
> The intrinsic `llvm.set_rounding` was introduced to facilitate work on IR, because IR is (mainly) target-agnostic and defines like `FE_TONEAREST` are not available there. It would be nice to have a C builtin counterpart for `FLT_ROUNDS`, I don't know why the standard does not define it.
>
> Implementation of `fesetround` with `set_rounding` would be very useful, but it requires translation of target-specific values for rounding mode into universal values and also checking availability of FPU, as `fesetround` returns value if rounding mode cannot be set. Both these checks are likely to require separate builtins.

Hi, @sepavloff 
This patch only aims to add a builtin for llvm.set.rounding, llvm.set.rounding can benefit C/C++ developers then. Since llvm.set.rounding is target-independent, __builtin_fesetround should be target-independent as well and the then arguments for this builtin should align with what documented here: https://llvm.org/docs/LangRef.html#id1502 for all targets. If we want to map fesetround to llvm.set.rounding in the future, we need to take care of 2 points:

1. translate fesetround arugment to llvm.set.rounding arugment.
2. properly handle the return value.

Did I correctly understand your points?
Thanks very much.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D144454/new/

https://reviews.llvm.org/D144454