Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[SYCL][NewPM] Add SYCLLowerWGLocalMemory pass into optimization #5301

Merged
merged 3 commits into from Jan 20, 2022
Merged

[SYCL][NewPM] Add SYCLLowerWGLocalMemory pass into optimization #5301

merged 3 commits into from Jan 20, 2022

Conversation

mlychkov
Copy link
Contributor

Add SYCLLowerWGLocalMemory pass into clang optimizations pipeline when using
new Pass Manager.

Signed-off-by: Mikhail Lychkov mikhail.lychkov@intel.com

@mlychkov
[SYCL][NewPM] Add SYCLLowerWGLocalMemory pass into optimization
61e6f9e 
Add SYCLLowerWGLocalMemory pass into clang optimizations pipeline when using
new Pass Manager.

Signed-off-by: Mikhail Lychkov <mikhail.lychkov@intel.com>
@mlychkov
Validate LIT test for both legacy and new PMs
ab39b5f 
Signed-off-by: Mikhail Lychkov <mikhail.lychkov@intel.com>
@mlychkov mlychkov requested a review from a team as a code owner January 13, 2022 12:00
smanna12
smanna12 previously approved these changes Jan 13, 2022
View reviewed changes
@mlychkov mlychkov requested a review from a team as a code owner January 13, 2022 14:03
@mlychkov
Copy link
Contributor Author

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

@AlexeySachkov
Copy link
Contributor

@intel/llvm-reviewers-runtime, could you please take a look at this PR? The only runtime change here is a small adjustment to a LIT test

steffenlarsen
steffenlarsen approved these changes Jan 20, 2022
View reviewed changes
Copy link
Contributor

@steffenlarsen steffenlarsen left a comment

Choose a reason for hiding this comment

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

Runtime changes LGTM!

@AlexeySachkov AlexeySachkov merged commit 25acbd2 into intel:sycl Jan 20, 2022
@bader
Copy link
Contributor

bader commented Jan 20, 2022

@mlychkov, this change broke (already broken ;)) build of shared libraries with clang.

/usr/bin/ld: lib/Target/NVPTX/CMakeFiles/LLVMNVPTXCodeGen.dir/NVPTXTargetMachine.cpp.o: in function LLVMInitializeNVPTXTarget': NVPTXTargetMachine.cpp:(.text.LLVMInitializeNVPTXTarget+0x86): undefined reference to llvm::initializeLocalAccessorToSharedMemoryPass(llvm::PassRegistry&)'

Looks like a missing dependency in cmake files.

@bader
Copy link
Contributor

bader commented Jan 21, 2022

@mlychkov corrected me this issue appeared after 58508ba.
@jchlanda, could you take a look, please?

@jchlanda
Copy link
Contributor

@mlychkov corrected me this issue appeared after 58508ba. @jchlanda, could you take a look, please?

@bader I might not get to it, as it's my last day before holiday, please feel free to revert [58508ba](58508ba if it's to blame and I'll look into it in a week's time, sorry.

@bader
Copy link
Contributor

bader commented Jan 21, 2022

Okay. I think it's the issue as reported here - 58508ba#commitcomment-64254777.
I'll try to look into it myself and if I'm unable to figure it out, I'll revert the commit.

@jchlanda
Copy link
Contributor

@mlychkov corrected me this issue appeared after 58508ba. @jchlanda, could you take a look, please?

Should be fixed in: #5357

@mlychkov mlychkov deleted the private/mlychkov/newPM_add_wg_loc_mem_pass branch January 25, 2022 09:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@AlexeySachkov AlexeySachkov AlexeySachkov approved these changes

@smanna12 smanna12 smanna12 approved these changes

@steffenlarsen steffenlarsen steffenlarsen approved these changes

@s-kanaev s-kanaev Awaiting requested review from s-kanaev s-kanaev is a code owner automatically assigned from intel/llvm-reviewers-runtime

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@mlychkov @AlexeySachkov @bader @jchlanda @smanna12 @steffenlarsen