This is an archive of the discontinued LLVM Phabricator instance.

[DebugInfo] Fix pr41180 : Loop Vectorization Debugify Failure
ClosedPublic

Authored by Orlando on Mar 28 2019, 9:46 AM.

Details

Summary

Bug: https://bugs.llvm.org/show_bug.cgi?id=41180

In the bug test case the debug location was missing for the cmp instruction in the "middle block" BB. This patch fixes the bug by copying the debug location from the cmp of the scalar loop's terminator branch, if it exists.

The patch also fixes the debug location on the subsequent branch instruction. It was previously using the location of the of the original loop's pre-header block terminator. Both of these instructions will now map to the source line of the conditional branch in the original loop.

A regression test has been added that covers these issues.

Diff Detail

Repository
rL LLVM

Event Timeline

Orlando created this revision.Mar 28 2019, 9:46 AM
aprantl added inline comments.Mar 29 2019, 9:46 AM
llvm/test/Transforms/LoopVectorize/middle-block-dbg.ll
5 ↗(On Diff #192655)

Could you please include the source code this was generated from in the comment in case we need to regenerate it later?

8 ↗(On Diff #192655)

please also check for the expected kind of instruction here.

Orlando updated this revision to Diff 193050.Apr 1 2019, 1:09 AM

I've updated the test to include the source and commands to generate the IR it uses. Please note that I stripped out the call void @llvm.dbg.* lines from the test file by hand so any regenerated IR will look more cluttered.

Orlando updated this revision to Diff 193055.Apr 1 2019, 2:17 AM

The test has been updated to check for the expected instructions.

Orlando marked 2 inline comments as done.Apr 1 2019, 2:18 AM
vsk accepted this revision.Apr 1 2019, 12:05 PM

Thanks, lgtm.

llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
2876 ↗(On Diff #193055)

Consider adding a comment here explaining that the original loop must have a single latch block, because we already require it to have a single exit block?

This revision is now accepted and ready to land.Apr 1 2019, 12:05 PM
Orlando updated this revision to Diff 193245.Apr 2 2019, 2:40 AM

Updated a comment with some more info as vsk suggested.

Please could someone commit this change for me? I do not have commit access.

Orlando marked an inline comment as done.Apr 2 2019, 2:40 AM
This revision was automatically updated to reflect the committed changes.