This is an archive of the discontinued LLVM Phabricator instance.

[OpenMP] Refactor the `target` function
AbandonedPublic

Authored by tianshilei1992 on Jul 28 2020, 10:59 AM.

Details

Reviewers
jdoerfert
ye-luo
Summary

Refactor the target function to make preparation of fixing the issue
of ahead-of-time device memory deallocation. This patch has following changes:

  1. Replaced all mutex lock/unlock with std::lock_guard;
  2. Renamed all variables to conform with LLVM code standard;
  3. Took data process out of the target function.

Diff Detail

Event Timeline

Herald added a project: Restricted Project. · View Herald TranscriptJul 28 2020, 10:59 AM
tianshilei1992 requested review of this revision.Jul 28 2020, 10:59 AM
ye-luo requested changes to this revision.Jul 28 2020, 11:30 AM

Needs to split this patch into three.

  1. function renaming. In addtion, should we update target_data_update as well?
  2. std::lock_guard change.
  3. "target" change.

The order of 1 and 2 can be flexible

This revision now requires changes to proceed.Jul 28 2020, 11:30 AM

I don't think it deserves three patches. The goal is to refactor the target function, and this patch just did this only thing. According to the bi-weekly meeting, the renaming could be with other related changes.

In addtion, should we update target_data_update as well?

I didn't touch target_data_update. Basically I only take care of related code.

I don't think it deserves three patches. The goal is to refactor the target function, and this patch just did this only thing. According to the bi-weekly meeting, the renaming could be with other related changes.

In addtion, should we update target_data_update as well?

I didn't touch target_data_update. Basically I only take care of related code.

I think if massive renaming files and variables significantly introduce challenges for code review on the relevant change. It is more appropriate to do the renaming separately.

tianshilei1992 abandoned this revision.Jul 28 2020, 1:53 PM

Will resubmit in three patches.