[PATCH] D62088: [compiler-rt][builtins] Scaled Integer log10()

[Leonard Chan via Phabricator via llvm-commits]

leonardchan added a comment.

In D62088#1537470 <https://reviews.llvm.org/D62088#1537470>, @bjope wrote:

> Please add test cases for scale=0 and scale=width as I assume those need special handling (UB right now?).
>  And if scale=0 and scale=width needs special handling, then I guess scale=1 and scale=width-1 are new boundary values so I maybe it would be nice to have tests for those scales as well.


Added. For a scale of 0 we only return the integer result. For widths > 60 or 28, we return INT_MAX to represent an error since we need to represent 10 << scale in 64/32 bits for this to work. So the scale boundaries are [0,60] for the 64 bit version and [0,28] for the 32 bit version. We could technically increase this to go up to 63/31, although to get the precise result would require a bigger buffer (using more 128 bit ints) which would complicate things further.

For the 64 bit function, I realized that in order to get the precise result without a 128 bit int would require implementing division by 10 in 2 64 bit ints. I eventually found a way to do this and get the precise result, but am not sure if it should be included in this patch since it would be adding another large function. Perhaps it could be readded in a followup? For now, I removed the functions we fallback to if 128 bit ints aren't supported and only define the function if 128 bits are enabled.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62088