Skip to content

Commit 7a027f3

Browse files
committed
[ntuple] Fix wrong assert in Real32Quant
The assert in QuantizeReals() should not assume that all values are in range, as we want to throw an exception at the end of the function if that is the case. Another mistake conspired to hide this bug from all non-windows builds: LeadingZeroes() and TrailingZeroes() were incorrectly using the 64-bit version of the intrinsics, where they should have used the 32-bit one. This means they effectively never fired as they ranged from 0 to 63 rather than the intended 0 to 31.
1 parent 1dc46e8 commit 7a027f3

File tree

1 file changed

+5
-4
lines changed

1 file changed

+5
-4
lines changed

tree/ntuple/src/RColumnElement.hxx

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -996,7 +996,7 @@ using Quantized_t = std::uint32_t;
996996
return static_cast<std::size_t>(31 - idx);
997997
return 32;
998998
#else
999-
return static_cast<std::size_t>(__builtin_clzl(x));
999+
return static_cast<std::size_t>(__builtin_clz(x));
10001000
#endif
10011001
}
10021002

@@ -1011,7 +1011,7 @@ using Quantized_t = std::uint32_t;
10111011
return static_cast<std::size_t>(idx);
10121012
return 32;
10131013
#else
1014-
return static_cast<std::size_t>(__builtin_ctzl(x));
1014+
return static_cast<std::size_t>(__builtin_ctz(x));
10151015
#endif
10161016
}
10171017

@@ -1042,14 +1042,15 @@ int QuantizeReals(Quantized_t *dst, const T *src, std::size_t count, double min,
10421042
for (std::size_t i = 0; i < count; ++i) {
10431043
const T elem = src[i];
10441044

1045-
nOutOfRange += !(min <= elem && elem <= max);
1045+
bool outOfRange = !(min <= elem && elem <= max);
1046+
nOutOfRange += outOfRange;
10461047

10471048
const double e = 0.5 + (elem - min) * scale;
10481049
Quantized_t q = static_cast<Quantized_t>(e);
10491050
ByteSwapIfNecessary(q);
10501051

10511052
// double-check we actually used at most `nQuantBits`
1052-
assert(LeadingZeroes(q) >= unusedBits);
1053+
assert(outOfRange || LeadingZeroes(q) >= unusedBits);
10531054

10541055
// we want to leave zeroes in the LSB, not the MSB, because we'll then drop the LSB
10551056
// when bit packing.

0 commit comments

Comments
 (0)