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

Conversation

vasilytric
Copy link

The test uses reference data and different alignment flags. Invokes simd constructors in different contexts with provided reference data and alignment flag.
It is expected for destination simd instance to store a bitwise same data as the reference one.

@vasilytric
Copy link
Author

CI on Windows platform failed due to issue, that is not related to this PR.
@kbobrovs Would you mind to take a look?


bool passed = true;
const auto types = get_tested_types<tested_types::all>();
const auto dims = get_all_dimensions();
Copy link

Choose a reason for hiding this comment

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

Nit for future: please rename dimensions to vector_lengths or sizes

Copy link
Author

Choose a reason for hiding this comment

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

dimensions will be changed to one of suggested names for all tests in the one of next PR`

// flag.
// It is expected for destination simd instance to store a bitwise same data as
// the reference one.

Copy link

Choose a reason for hiding this comment

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

Nits (can be added later)

  • accessor-based load constructor not tested? If tested elsewhere, this should be called smth like usm_ctor*...
  • most popular alignment for loading as a block will be overaligned<16>{} - oword. This is minimum requirement for block load. Not tested.

Copy link
Author

Choose a reason for hiding this comment

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

accessor-based load constructor not tested

We are going to add tests on accessor-based load constructor in separate PR

most popular alignment for loading as a block will be overaligned<16>{} - oword. This is minimum requirement for block load. Not tested.

For overaligned we are using std::max_align_t that means we will use synonymous with the largest scalar type, which is long double on most platforms.
In the test we defined it here
Launch tests with this alignment type here

Copy link

Choose a reason for hiding this comment

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

For overaligned we are using std::max_align_t that means we will use synonymous with the largest scalar type, which is long double on most platforms.

we don't really care about any other platforms here, and long double is not a native type in Intel GPUs. So 16 is not driven by any type, but rather the "oword alignment" requirement for all block loads. In that sense, std::max_align_t gives wrong idea.

Copy link

Choose a reason for hiding this comment

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

so please extend the testcases with oword alignment.

Copy link
Author

Choose a reason for hiding this comment

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

Alignment will be changed in one of next PRs

@vasilytric
Copy link
Author

@kbobrovs All verifications has passed. Could you merge this PR?

@kbobrovs kbobrovs merged commit 1548e68 into intel:intel Feb 2, 2022
myler pushed a commit to myler/llvm-test-suite that referenced this pull request Apr 12, 2022
myler pushed a commit to myler/llvm-test-suite that referenced this pull request Jun 17, 2022
aelovikov-intel pushed a commit to aelovikov-intel/llvm that referenced this pull request Mar 27, 2023
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