This is an archive of the discontinued LLVM Phabricator instance.

[Polly][DependenceInfo] Simplify creation and subsequent use of AccessSchedule
ClosedPublic

Authored by bollu on Feb 20 2017, 1:23 PM.

Diff Detail

Repository
rL LLVM

Event Timeline

bollu created this revision.Feb 20 2017, 1:23 PM

Dear Siddharth,

thanks for the patch. The changes itself look very good, however we should probably still update the comment above the lines you change. Also, it might sense to rename AccessSchedule to ReductionTagMap. After this patch, AccessSchedule does not contain any schedule any more.

Also:

  • Please mark patches that do not change functionality with a '[NFC]' tag at the end of the title line.
  • Add me also as a reviewer (the command I gave you did not add my own name)
  • Add [Polly] at the beginning of the subject line, such that people know for which project this patch is for.

Please update the patch (use --update flag in arc). I will then go over it once more, and commit it if no other changes are needed.

grosser added a project: Restricted Project.Feb 21 2017, 12:25 AM
bollu retitled this revision from [DependenceInfo] Simplify creation and subsequent use of AccessSchedule to [Polly][DependenceInfo] Simplify creation and subsequent use of AccessSchedule.Feb 21 2017, 5:38 AM
bollu added a reviewer: grosser.
bollu updated this revision to Diff 89195.EditedFeb 21 2017, 6:03 AM

Updating D30179: [Polly][DependenceInfo] Simplify creation and subsequent use of AccessSchedule

renamed AccessSchedule to ReductionTagMap, edited the comment in collectInfo to reflect what is happening after the change.

grosser edited edge metadata.Feb 21 2017, 6:05 AM

Good. Just one minor comment also needs updating.

Also please add '[NFC]' into the subject line of this change.

lib/Analysis/DependenceInfo.cpp
331 ↗(On Diff #89195)

This comment still needs to be updated.

bollu updated this revision to Diff 89210.Feb 21 2017, 7:31 AM

update comment that explained what ReductionTags does and how it is created.

bollu updated this revision to Diff 89213.Feb 21 2017, 7:41 AM

collecting all diffs together into a patch.

This revision was automatically updated to reflect the committed changes.