Skip to content
This repository was archived by the owner on May 9, 2024. It is now read-only.

Don't use streaming-top-n in intel gpu case #455

Merged
merged 1 commit into from
May 8, 2023

Conversation

kurapov-peter
Copy link
Contributor

This fixes the last code generation-related assertion failure we have in the L0 case.

@@ -907,10 +907,14 @@ std::unique_ptr<QueryMemoryDescriptor> build_query_memory_descriptor(
case QueryDescriptionType::Projection: {
CHECK(!must_use_baseline_sort);

bool streaming_top_n_supported_by_platform =
device_type == ExecutorDeviceType::CPU ||
executor->getDataMgr()->getGpuMgr()->getPlatform() != GpuMgrPlatform::L0;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe getGpuMgr can return nullptr.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, thanks!

@kurapov-peter kurapov-peter force-pushed the pakurapo/streaming-top-n branch from 07ae22a to ecaf20e Compare May 3, 2023 20:08
@@ -907,10 +907,15 @@ std::unique_ptr<QueryMemoryDescriptor> build_query_memory_descriptor(
case QueryDescriptionType::Projection: {
CHECK(!must_use_baseline_sort);

bool streaming_top_n_supported_by_platform =
device_type == ExecutorDeviceType::CPU ||
(executor->getDataMgr() && executor->getDataMgr()->getGpuMgr() &&
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are playing too safe, data_mgr has to be non-null

@kurapov-peter kurapov-peter merged commit 5a10e57 into main May 8, 2023
@kurapov-peter kurapov-peter deleted the pakurapo/streaming-top-n branch May 8, 2023 12:45
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants