Skip to content

[SYCL][NewPM] Disable vectorization and loop transformation #5305

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Conversation

mlychkov
Copy link
Contributor

Port changes that disable vectorization and loop transformation passes
in optimization pipeline from legacy Pass Manager builder to new PM builder
(see commit ff6929e).
Also for those tests that fail without this fix after optimization by
the new Pass Manager add validation for both legacy and new PMs.

Signed-off-by: Mikhail Lychkov [email protected]

Port changes that disable vectorization and loop transformation passes
in optimization pipeline from legacy Pass Manager builder to new PM builder
(see commit ff6929e).
Also for those tests that fail without this fix after optimization by
the new Pass Manager add validation for both legacy and new PMs.

Signed-off-by: Mikhail Lychkov <[email protected]>
Signed-off-by: Mikhail Lychkov <[email protected]>
@mlychkov mlychkov requested review from a team as code owners January 13, 2022 15:29
@mlychkov mlychkov requested a review from bader January 13, 2022 15:31
@mlychkov
Copy link
Contributor Author

The full set of fixes for new PM also validated in #4860.

bader
bader previously approved these changes Jan 13, 2022
// RUN: %clangxx -I %sycl_include %s -o %t.out -fsycl -O0 -g
// Check that the code compiles with -O0 and -g on both legacy and new Pass
// Managers
// RUN: %clangxx -I %sycl_include %s -o %t.out -fsycl -fno-legacy-pass-manager -O0
Copy link
Contributor

Choose a reason for hiding this comment

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

why this is added to esimd tests? How these 3 tests were selected for such modification?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

They crash on SYCLLowerESIMD pass execution step if loop transformation and vectorization passes are enabled in CodeGen.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@kbobrovs Could you please provide feedback or approve the PR if there are no more concerns?

Copy link
Contributor

Choose a reason for hiding this comment

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

I see. @mlychkov - could you please instead mark them as XFAIL, add a TODO and file an internal ticket?

Copy link
Contributor

Choose a reason for hiding this comment

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

@kbobrovs, the point is that those tests fail with new pass manager without changes proposed by @mlychkov, but they do pass with this PR using both pass managers

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok, I think I get it now. Thanks!

@AlexeySachkov
Copy link
Contributor

@mlychkov, could you please resolve merge conflicts?

@mlychkov
Copy link
Contributor Author

mlychkov commented Jan 26, 2022

Precommit tests failed due to CI infrastructure issue. Corresponding issue was raised already.

@bader
Copy link
Contributor

bader commented Jan 31, 2022

@intel/dpcpp-tools-reviewers, ping.

@bader bader merged commit 375d213 into intel:sycl Jan 31, 2022
@mlychkov
Copy link
Contributor Author

The issue about failed pre-commit on Windows has been raised.

@mlychkov mlychkov deleted the private/mlychkov/newPM_disable_vec_and_loop_passes_for_sycl branch January 31, 2022 11:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants