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

[Tim Shen via Phabricator via llvm-commits]

timshen added inline comments.


================
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);
----------------
sanjoy wrote:
> 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?
Done. Added rounding guarantee to udiv and sdiv.


================
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,
----------------
sanjoy wrote:
> Nit: rounding
Removed. Those should be decl comments, not def comments.


https://reviews.llvm.org/D48498