[PATCH] D48498: [APInt] Add helpers for rounding u/sdivs.

[Sanjoy Das via Phabricator via llvm-commits]

sanjoy accepted this revision.
sanjoy added a comment.
This revision is now accepted and ready to land.

lgtm



================
Comment at: llvm/include/llvm/ADT/APInt.h:2160
 
+/// Return A unsign-divided by B, rounded by the given roundining mode.
+APInt RoundingUDiv(const APInt &A, const APInt &B, APInt::Rounding RM);
----------------
timshen wrote:
> sanjoy wrote:
> > Nit: rounding
> > 
> > Did you consider implementing these in `sdiv` and `udiv` and passing in a defaulted `Rounded` parameter?  That seems a bit cleaner to me.
> > 
> > Otherwise can you please add comments on `sdiv` and `udiv` about their rounding behavior?
> > Did you consider implementing these in sdiv and udiv and passing in a defaulted Rounded parameter? That seems a bit cleaner to me.
> 
> I thought about it, but it seems bad to just add complexity to the current APInt API, especially when we didn't find more usage.
> 
> APIntOps, on the other hand, are considered by me a set of helpers built on the top of APInt, therefore not adding API complexity to APInt itself.
> 
> > Otherwise can you please add comments on sdiv and udiv about their rounding behavior?
> 
> I'm not sure if the behavior is intentionally not promised to the user, or just undocumented.
> APIntOps, on the other hand, are considered by me a set of helpers built on the top of APInt, therefore not adding API complexity to APInt itself.

SGTM

> I'm not sure if the behavior is intentionally not promised to the user, or just undocumented.

But you're relying on it right, when the users requests TOWARDS_ZERO?


================
Comment at: llvm/lib/Support/APInt.cpp:2680
+
+/// Return A sign-divided by B, rounded by the given roundining mode.
+APInt llvm::APIntOps::RoundingSDiv(const APInt &A, const APInt &B,
----------------
Nit: rounding


https://reviews.llvm.org/D48498