This is an archive of the discontinued LLVM Phabricator instance.

Add end location of loop to loop metadata.
ClosedPublic

Authored by fhahn on Oct 19 2016, 4:31 AM.

Details

Summary

The patch adds the loop end location to the loop metadata. This additional information can be used to improve the locations when generating remarks for loops.

The patch depends on the companion LLVM patch D25763 (https://reviews.llvm.org/D25763).

Diff Detail

Event Timeline

fhahn updated this revision to Diff 75125.Oct 19 2016, 4:31 AM
fhahn retitled this revision from to Add end location of loop to loop metadata..
fhahn updated this object.
fhahn added a reviewer: hfinkel.
fhahn added a subscriber: cfe-commits.
fhahn added a comment.Nov 7 2016, 2:51 PM

Ping. It would be great if somebody could have a look at this patch. The companion LLVM patch D25763 (https://reviews.llvm.org/D25763) has been accepted already.

hfinkel edited edge metadata.

This makes sense to me; @anemet , @rjmccall , any thoughts?

rjmccall edited edge metadata.Nov 7 2016, 4:53 PM

This makes sense to me; @anemet , @rjmccall , any thoughts?

Why are the constructor arguments optional? Are there actually any loops that we push that lack start and end locs?

fhahn updated this revision to Diff 77317.Nov 9 2016, 1:36 AM
fhahn edited edge metadata.
fhahn added a comment.Nov 9 2016, 1:39 AM

@rjmccall thanks for the feedback. I initially kept the arguments optional because the single Location argument used to be optional as well.

But I think we can pass proper locations for all loops constructed by clang and I updated my patch to make the Start and End locations non-optional, which in turn required passing them through when constructing loops in lib/CodeGen/CGStmtOpenMP.cpp .

rjmccall accepted this revision.Nov 9 2016, 6:19 PM
rjmccall edited edge metadata.

Looks good, thanks!

This revision is now accepted and ready to land.Nov 9 2016, 6:19 PM
This revision was automatically updated to reflect the committed changes.