[PATCH] D84159: GlobalISel: Handle llvm.roundeven

[Matt Arsenault via Phabricator via llvm-commits]

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


================
Comment at: llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp:2461
+    // operations, round and roundeven are the same operatio.
+    changeOpcode(MI, TargetOpcode::G_INTRINSIC_ROUND);
+    return Legalized;
----------------
sepavloff wrote:
> arsenm wrote:
> > sepavloff wrote:
> > > arsenm wrote:
> > > > sepavloff wrote:
> > > > > I am afraid this change breaks the semantics of `roundeven`. Intrinsic `round` implements the same operation as libm function `round` (http://llvm.org/docs/LangRef.html#id554). In the latest draft (http://www.open-std.org/jtc1/sc22/wg14/www/docs/n2478.pdf, 7.12.9.6p2) the function `round` is described:
> > > > > ```
> > > > > The round functions round their argument to the nearest integer value in floating-point format,
> > > > > rounding halfway cases away from zero, regardless of the current rounding direction.
> > > > > ```
> > > > > In the same draft `roundeven` is described (7.12.9.8p2):
> > > > > ```
> > > > > The roundeven functions round their argument to the nearest integer value in floating-point format,
> > > > > rounding halfway cases to even (that is, to the nearest value that is an even integer), regardless of
> > > > > the current rounding direction.
> > > > > ```
> > > > > `round` and `roundeven` implement different rounding modes. Both functions do not depend on the current rounding mode.
> > > > > 
> > > > > LIBC variants provide `roundeven` but these implementations look complicated. There must be an algorithm which uses `trunc` and implements `roundeven` suitable for vector operations. Probably the algorithm that uses `remainder` in https://stackoverflow.com/questions/32746523/ieee-754-compliant-round-half-to-even can be used to implement `roundeven` similar to `round` in `LegalizerHelper::lowerIntrinsicRound`.
> > > > > 
> > > > Oh right, this should have been G_FRINT. I can't keep all the rounding functions straight
> > > Why not adding new target opcode, like G_FROUNDEVEN or G_INTRINSIC_ROUNDEVEN?
> > That is added, and this is where it's lowered to the other equivalent operation
> Indeed :)
> What is the purpose of adding such replacement? In general, it is invalid, as it may be applied only in default FP mode. Using library calls is a safer solution. IIUC, if some target is required to support `roundeven` it must either implement custom lowering or appropriate libc must be used. Are there any reasons to allow such incomplete solution?
AMDGPU has no library calls and does have the instruction. The unconstrained FP operations are defined as running in the default FP mode. It's quite undefined to execute them in another mode,  these are equivalent operations.


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

https://reviews.llvm.org/D84159