[PATCH] D54719: [Intrinsic] Signed Fixed Point Multiplication Intrinsic

[Bjorn Pettersson via Phabricator via llvm-commits]

bjope added inline comments.


================
Comment at: llvm/docs/LangRef.rst:12830
+value is rounded up or down to the closest representable value. The rounding
+direction is unspecified.
+
----------------
ebevhan wrote:
> efriedma wrote:
> > "The rounding direction is unspecified" seems scary to me... is there really no standard for rounding these operations?
> The Embedded-C standard says that the rounding direction is implementation-defined, and is allowed to be indeterminable. 
> 
> I also think it's a bit unfortunate that we can't express the rounding direction, since it inhibits optimization, but locking it down to either rounding up or down will make things annoying for any target with operations that round the other direction.
> 
> Saying that it's unspecified lets us properly legalize this to something sensible while allowing targets to implement it with legal operations if they have them.
> 
> For what it's worth, the legalization will make it round down (toward negative infinity).
Maybe it should say "target specific" instead? And then the legalization (and any constant folding etc) could use some target transform hook to decide in which direction to do the rounding (I guess it could be set to up/down/any).

Does "unspecified" mean that ConstantFolding use one strategy for rounding and legalization another strategy? Or should we only allow constant folding when rounding isn't a problem (such as multiply by zero)?

Things could still be annoying for a target that does care about rounding. Or maybe it isn't allowed for a target to decide what the implementation-defined behavior should be? Or should it be the same for all targets?

A first implementation could of course go for only supporting "unspecified" (if it is clear what kind of constant folding and other optimizations that are allowed). If a target needs a "target specific" behavior, then I assume the code could be sprinkled with hooks at a later stage.



Repository:
  rL LLVM

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

https://reviews.llvm.org/D54719