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

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added inline comments.


================
Comment at: llvm/docs/LangRef.rst:12931-12934
+'``llvm.roundeven.*``' Intrinsic
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+Syntax:
----------------
sepavloff wrote:
> 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.
> 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.

This is already very much the case. The regular FP operations and non constrained intrinsics assume the default FP environment



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75670