Skip to content

Commit

Permalink
Fix crash when SimdComparison calls SIMD instructions (#8820)
Browse files Browse the repository at this point in the history
Summary:
Fixes #8668

Pull Request resolved: #8820

Reviewed By: Yuhta

Differential Revision: D54454632

Pulled By: kgpai

fbshipit-source-id: 08f6f98f151c2a7c2b80f6b414159ce49a52faca
  • Loading branch information
xiaodai1002 authored and facebook-github-bot committed Mar 5, 2024
1 parent 9300329 commit 32b9cad
Show file tree
Hide file tree
Showing 2 changed files with 35 additions and 1 deletion.
2 changes: 1 addition & 1 deletion velox/functions/prestosql/Comparisons.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ struct SimdComparator {
if constexpr (numScalarElements == 2 || numScalarElements == 4) {
for (auto i = begin; i < vectorEnd; i += 8) {
rawResult[i / 8] = 0;
for (auto j = 0; j < 8 && j < vectorEnd; j += numScalarElements) {
for (auto j = 0; j < 8 && (i + j) < vectorEnd; j += numScalarElements) {
auto left = loadSimdData<T, isLeftConstant>(rawLhs, i + j);
auto right = loadSimdData<T, isRightConstant>(rawRhs, i + j);

Expand Down
34 changes: 34 additions & 0 deletions velox/functions/prestosql/tests/ComparisonsTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -601,6 +601,40 @@ TEST_F(ComparisonsTest, eqNestedComplex) {
}
}

TEST_F(ComparisonsTest, overflowTest) {
auto makeFlatVector = [&](size_t numRows, size_t delta) {
BufferPtr values =
AlignedBuffer::allocate<int64_t>(numRows + delta, pool());
auto rawValues = values->asMutable<int64_t>();
for (size_t i = 0; i < numRows + delta; ++i) {
rawValues[i] = i;
}
return std::make_shared<FlatVector<int64_t>>(
pool(), BIGINT(), nullptr, numRows, values, std::vector<BufferPtr>{});
};

size_t numRows = 1006;
size_t delta = 2;
auto rowVector = makeRowVector(
{makeFlatVector(numRows, delta), makeFlatVector(numRows, delta)});
auto result =
evaluate<SimpleVector<bool>>(fmt::format("{}(c0, c1)", "eq"), rowVector);
for (auto i = 0; i < result->size(); ++i) {
ASSERT_TRUE(result->valueAt(i));
}

auto flatResult = result->asFlatVector<bool>();
auto rawResult = flatResult->mutableRawValues();
for (auto i = 0; i < result->size(); ++i) {
ASSERT_TRUE(
bits::isBitSet(reinterpret_cast<const uint64_t*>(rawResult), i));
}
for (auto i = result->size(); i < result->size() + delta; ++i) {
ASSERT_FALSE(
bits::isBitSet(reinterpret_cast<const uint64_t*>(rawResult), i));
}
}

namespace {
template <typename Tp, typename Op, const char* fnName>
struct ComparisonTypeOp {
Expand Down

0 comments on commit 32b9cad

Please sign in to comment.