Skip to content
This repository was archived by the owner on Mar 28, 2023. It is now read-only.

[SYCL][ESIMD] Lsc predicate test #1194

Merged
merged 5 commits into from
Sep 9, 2022
Merged

[SYCL][ESIMD] Lsc predicate test #1194

merged 5 commits into from
Sep 9, 2022

Conversation

fineg74
Copy link

@fineg74 fineg74 commented Sep 1, 2022

Complementary PR for: intel/llvm#6688

@fineg74 fineg74 requested a review from a team as a code owner September 1, 2022 19:59
@fineg74 fineg74 changed the title [SYCL][ESIMD} Lsc predicate test [SYCL][ESIMD] Lsc predicate test Sep 1, 2022
@fineg74
Copy link
Author

fineg74 commented Sep 2, 2022

/verify with intel/llvm#6688

@@ -0,0 +1,200 @@
//==------------ lsc_neg.cpp - DPC++ ESIMD on-device test ------------------==//
Copy link

Choose a reason for hiding this comment

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

wrong filename

Copy link
Author

Choose a reason for hiding this comment

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

Fixed

// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
//
//===----------------------------------------------------------------------===//
// REQUIRES: gpu-intel-pvc || esimd_emulator
Copy link

Choose a reason for hiding this comment

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

please add description what the test does

Copy link
Author

Choose a reason for hiding this comment

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

Fixed

//
//===----------------------------------------------------------------------===//
// REQUIRES: gpu-intel-pvc || esimd_emulator
// UNSUPPORTED: cuda || hip
Copy link

Choose a reason for hiding this comment

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

this line is not needed

Copy link
Author

Choose a reason for hiding this comment

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

Fixed

//===----------------------------------------------------------------------===//
// REQUIRES: gpu-intel-pvc || esimd_emulator
// UNSUPPORTED: cuda || hip
// RUN: %clangxx -fsycl %s -o %t.out
Copy link

Choose a reason for hiding this comment

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

please create a *_stateless variant of the test - see e.g. llvm-test-suite/SYCL/ESIMD/lsc/lsc_surf_stateless.cpp
(use -fsycl-esimd-force-stateless-mem to compile)
Otherwise this test does not catch a bug in the predicate implementation PR (intel/llvm#6688)

Copy link
Author

Choose a reason for hiding this comment

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

Done

lsc_block_store<int, SIMDSize>(access_0, offset, data_0 * 2,
pred_enable);

lsc_prefetch<int, SIMDSize, lsc_data_size::default_size,
Copy link

Choose a reason for hiding this comment

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

please try to use only minimal set of APIs. lsc_prefetch is not needed.

Copy link
Author

Choose a reason for hiding this comment

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

Done

offset);
auto data_2 =
lsc_block_load<int, SIMDSize>(access_2, offset, pred_enable);
lsc_block_store<int, SIMDSize>(access_2, offset, data_2 * 2,
Copy link

Choose a reason for hiding this comment

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

we have 4 loads and 4 stores - do they test some different aspects of the API? If not, please minimize.

Copy link
Author

Choose a reason for hiding this comment

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

I tried to verify all possible combinations of predicate flags. Not sure it is strictly necessary but won't hurt.


int testUSM(queue q) {
auto size = size_t{128};
auto constexpr SIMDSize = unsigned{4};
Copy link

Choose a reason for hiding this comment

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

I think this is not a practical SIMD size. 8,16,32 would be more relevant. Or, even better, you can templatize by SIMD size and check all of them - like we usually do in other tests.

Copy link
Author

Choose a reason for hiding this comment

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

Done

Copy link

@kbobrovs kbobrovs left a comment

Choose a reason for hiding this comment

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

Please address comments.

@kbobrovs kbobrovs merged commit 288acc6 into intel:intel Sep 9, 2022
@fineg74 fineg74 deleted the lscPredicateTest branch September 9, 2022 18:07
myler pushed a commit to myler/llvm-test-suite that referenced this pull request Mar 22, 2023
* Add test for lsc block load/store "whole operation" predicates.
myler pushed a commit to myler/llvm-test-suite that referenced this pull request Mar 22, 2023
[SYCL][ESIMD] Lsc block load/store predicate test (intel#1194)
aelovikov-intel pushed a commit to aelovikov-intel/llvm that referenced this pull request Mar 27, 2023
…ite#1194)

* Add test for lsc block load/store "whole operation" predicates.
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.

2 participants