This is an archive of the discontinued LLVM Phabricator instance.

Look for a loop's starting location in the llvm.loop metadata
ClosedPublic

Authored by hfinkel on Apr 29 2016, 12:41 PM.

Details

Summary

Getting accurate locations for loops is important, because those locations are used by the frontend to generate optimization remarks. Currently, optimization remarks for loops often appear on the wrong line, often the first line of the loop body instead of the loop itself. This is confusing because that line might itself be another loop, or might be somewhere else completely if the body was inlined function call. This happens because of the way we find the loop's starting location. First, we look for a preheader, and if we find one, and its terminator has a debug location, then we use that. Otherwise, we look for a location on an instruction in the loop header.

The fallback heuristic is not bad, but will almost always find the beginning of the body, and not the loop statement itself. The preheader location search often fails because there's often not a preheader, and even when there is a preheader, depending on how it was formed, it sometimes carries the location of some preceeding code.

I don't see any good theoretical way to fix this problem. On the other hand, this seems like a straightforward solution: Put the debug location in the loop's llvm.loop metadata. A companion Clang patch will cause Clang to insert llvm.loop metadata with appropriate locations when generating debugging information. With these changes, our loop remarks have much more accurate locations.

Diff Detail

Repository
rL LLVM

Event Timeline

hfinkel updated this revision to Diff 55645.Apr 29 2016, 12:41 PM
hfinkel retitled this revision from to Look for a loop's starting location in the llvm.loop metadata.
hfinkel updated this object.
hfinkel added reviewers: dblaikie, echristo, anemet.
hfinkel added a subscriber: llvm-commits.
hfinkel updated this revision to Diff 55874.May 2 2016, 1:04 PM
hfinkel added a reviewer: bogner.

Updated based on Justin's feedback.

anemet accepted this revision.May 10 2016, 5:39 PM
anemet edited edge metadata.

LGTM, some minor suggestions below.

lib/Analysis/LoopInfo.cpp
318 ↗(On Diff #55874)

Capitalize the variables?

test/Transforms/LoopVectorize/X86/vectorization-remarks-loopid-dbg.ll
5–7 ↗(On Diff #55874)

Here, are you just making sure that the location didn't leak into the generated debug info? Perhaps a comment would help.

This revision is now accepted and ready to land.May 10 2016, 5:39 PM
This revision was automatically updated to reflect the committed changes.