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.

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

[Flang][OpenMP] Add support for lastprivate clause lowering. #1593

Open
wants to merge 1 commit into
base: fir-dev
Choose a base branch
from

Conversation

arnamoy10
Copy link

No description provided.

Copy link
Collaborator

@kiranchandramohan kiranchandramohan left a comment

Choose a reason for hiding this comment

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

Thanks @arnamoy10 for working on lastprivate.
Could you add an explanation for the approach that you have taken? Particularly, how is it ensured that it is the last iteration's value that is copied back to the original variable?

@arnamoy10
Copy link
Author

arnamoy10 commented Apr 19, 2022

Thanks for the comment @kiranchandramohan . From this C++ OpenMP example, looking at the diff between with and without the use of lastprivate, it looked like the change necessary was just to add a load and store of the lastprivate variable at the end of the loop. I did not see any code that checks the last iteration's value. I was under the impression that OpenMP runtime calls will take care of that.

It will be great if you can point to the part of the IR that ensures it.

@kiranchandramohan
Copy link
Collaborator

@arnamoy10 Can you read the discussion in https://discourse.llvm.org/t/rfc-privatisation-in-openmp-dialect/3526?

I think the lastprivate handling is not done in the body of the loop in the llvm it generated but it is in the footer or exit portion of the loop. We do not have a representation for the footer/exit portion of the loop in omp.wsloop. So we are forced to add a condition inside the body of the loop that checks whether it is the last iteration and then do an update.

I can explain more if required.

@schweitzpgi
Copy link

Q: is there a related phabricator review to this PR? Thanks for working on this.

@arnamoy10
Copy link
Author

@schweitzpgi Thanks for asking, unfortunately, privatization implementation, for now, has to be done in fir-dev, through pull req style. Here is the relevant discussion in slack

@kiranchandramohan
Copy link
Collaborator

Q: is there a related phabricator review to this PR? Thanks for working on this.

@schweitzpgi Is a phabricator review what you would prefer for OpenMP work?

@schweitzpgi
Copy link

Q: is there a related phabricator review to this PR? Thanks for working on this.

@schweitzpgi Is a phabricator review what you would prefer for OpenMP work?

See Steve's announcement here. https://discourse.llvm.org/t/nvidia-transition-from-fir-dev/61947

I think transitioning to working upstream, in so far as it is possible to do so, would be a good direction.

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.

3 participants