This is an archive of the discontinued LLVM Phabricator instance.

[LoopIdiomRecognize] @llvm.dbg values shouldn't affect the transformation.
ClosedPublic

Authored by davide on Feb 1 2019, 4:15 PM.

Diff Detail

Event Timeline

davide created this revision.Feb 1 2019, 4:15 PM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 1 2019, 4:15 PM
Herald added a subscriber: hiraditya. · View Herald Transcript
davide marked an inline comment as done.Feb 1 2019, 4:20 PM
davide added inline comments.
llvm/test/Transforms/LoopIdiom/dbginfo-cost.ll
2

this is a little annoying, but the transformation seems to be impacted by the intrinsic cost, and I wasn't able to generate a test that doesn't involve target specific bits.

LGTM with a little simpler testcase

llvm/lib/Transforms/Scalar/LoopIdiomRecognize.cpp
1467

I would be surprised if this is the only occurrence of this exact bug...

llvm/test/Transforms/LoopIdiom/dbginfo-cost.ll
47

It looks like we could safely strip out most of the debug info by removing enums: and friends, and making all !dbg attachments point to the same source location.

LGTM with a little simpler testcase

llvm/lib/Transforms/Scalar/LoopIdiomRecognize.cpp
1467

We should probably grep for "cost" in the optimizer.

jonpa added a subscriber: jonpa.Feb 1 2019, 6:06 PM
jonpa added inline comments.Feb 2 2019, 9:32 AM
llvm/lib/Transforms/Scalar/LoopIdiomRecognize.cpp
1467

Found another one, or at least related, I think: https://bugs.llvm.org/show_bug.cgi?id=40573

diff stat says I cleaned the test case a bit.

dtdebugger:llvm-mono davide$ git diff |diffstat
 dbginfo-cost.ll |   50 ++++++++------------------------------------------
 1 file changed, 8 insertions(+), 42 deletions(-)

Commit this in a second.

davide updated this revision to Diff 184973.Feb 3 2019, 12:32 PM

adrian's comments

This revision was not accepted when it landed; it landed in state Needs Review.Feb 3 2019, 12:33 PM
This revision was automatically updated to reflect the committed changes.