HappenLee commented on code in PR #67872:
URL: https://github.com/apache/doris/pull/67872#discussion_r3999373217
##########
be/src/exprs/math_functions.cpp:
##########
@@ -133,26 +133,30 @@ StringRef MathFunctions::decimal_to_base(FunctionContext*
ctx, int64_t src_num,
}
bool MathFunctions::decimal_in_base_to_decimal(int64_t src_num, int8_t
src_base, int64_t* result) {
- uint64_t temp_num = std::abs(src_num);
- int32_t place = 1;
- *result = 0;
+ auto magnitude = static_cast<uint64_t>(src_num);
+ if (src_num < 0) {
+ magnitude = 0 - magnitude;
+ }
+ uint64_t divisor = 1;
+ while (magnitude / divisor >= 10) {
Review Comment:
The initial scan still performs an integer division for each additional
decimal digit. We can make the divisor constant by scanning a shrinking copy of
`magnitude`:
```cpp
uint64_t divisor = 1;
uint64_t remaining = magnitude;
while (remaining >= 10) {
remaining /= 10;
divisor *= 10;
}
```
I checked the generated x86-64 assembly with Clang 16.0.6, Clang 20.1.8, and
GCC 15.1.0 using `-std=c++20 -O3 -DNDEBUG -msse4.2 -mavx2`. For both isolated
loops and an extracted copy of the complete conversion helper, all three
compilers retain a hardware `div` in the original scan, while the rewritten
scan uses multiplication and a shift for division by 10. For example, Clang 16
emits these operations:
```asm
# Original scan, 64-bit division path:
mov rax, rdi
xor edx, edx
div rcx
# Rewritten scan (constant loaded outside the loop):
# rsi = 0xCCCCCCCCCCCCCCCD
mov rax, rdi
mul rsi
shr rdx, 3
```
The compiler already eliminates the initial division by 1, so an input with
d decimal digits executes d - 1 divisions in the original scan. The proposed
change removes those divisions while preserving the computed highest-place
divisor, including for zero and the magnitude of INT64_MIN.
This is an assembly-confirmed optimization opportunity; I have not measured
the end-to-end SQL performance impact.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
