[PATCH] D75670: [FPEnv] Intrinsic llvm.roundeven

[Serge Pavlov via Phabricator via llvm-commits]

sepavloff marked an inline comment as done.
sepavloff added inline comments.


================
Comment at: llvm/docs/LangRef.rst:12931-12934
+'``llvm.roundeven.*``' Intrinsic
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+Syntax:
----------------
efriedma wrote:
> arsenm wrote:
> > efriedma wrote:
> > > arsenm wrote:
> > > > Isn't this the same as rint? Since the default rounding mode is assumed 
> > > The name is a hint that we should prefer to lower to a library function named "roundeven", if we can't generate an inline sequence.  Other than that, yes, it's identical.
> > > 
> > > I vaguely recall that I tried to merge llvm.rint and llvm.nearbyint at some point, but someone ran into issues with missing symbols.  I can try to dig it up if you think it's important.
> > I don't think the intrinsic name should ever influence codegen that way. We should not have redundant intrinsics, especially for some weird lowering hint
> See https://reviews.llvm.org/rL299247 for recent history of this.
> Isn't this the same as rint? Since the default rounding mode is assumed

`roundeven` does not assume any mode. It always rounds to nearest, ties to even.
If we eventually accept the viewpoint that default mode may be assumed always in the absence of `pragma FENV_ACCESS`, then it is more natural to replace `rint` with this intrinsic.

> The name is a hint that we should prefer to lower to a library function named "roundeven", if we can't generate an inline sequence. Other than that, yes, it's identical.

There is a library function `roundeven`, which of course differs from any other rounding function.

> I don't think the intrinsic name should ever influence codegen that way. We should not have redundant intrinsics, especially for some weird lowering hint

This is a function defined in the recent C standard draft http://www.open-std.org/jtc1/sc22/wg14/www/docs/n2454.pdf, 7.12.9.8.

> See https://reviews.llvm.org/rL299247 for recent history of this.

It is semantically necessary change as `rint` and `nearbyint` are different functions if we are interested with FP exception status.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75670