This is an archive of the discontinued LLVM Phabricator instance.

Have stripDebugInfo() also strip !llvm.loop annotations.
ClosedPublic

Authored by aprantl on Feb 5 2021, 2:21 PM.

Details

Summary

The !llvm.loop annotations consist of pointers into the debug info, so when stripping the debug info (particularly important when it is malformed!) !llvm.loop annotations need to be stripped as well, or else the malformed debug info stays around.

rdar://73687049

Diff Detail

Event Timeline

aprantl created this revision.Feb 5 2021, 2:21 PM
aprantl requested review of this revision.Feb 5 2021, 2:21 PM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 5 2021, 2:21 PM
vsk added inline comments.Feb 5 2021, 2:59 PM
llvm/lib/IR/DebugInfo.cpp
344

We should consider keeping the non-debug info metadata within MD_loop to avoid affecting optimization or opt-remarks. I think we can do that with I.setMD(MD_loop, stripDebugLocFromLoopID(MD)).

llvm/test/Verifier/llvm.loop-cu-strip.ll
1

Can you round-trip this through llvm-dis so we can check that the !llvm.loop metadata sticks around (I'm imagining some CHECK: !5 = distinct !{!5}).

vsk added a comment.Feb 5 2021, 3:02 PM

Or, if MD_loop without the debug info doesn't serve any purpose, it'd be fine to drop it entirely. Perhaps @fhahn can speak to this?

aprantl added inline comments.Feb 5 2021, 3:07 PM
llvm/lib/IR/DebugInfo.cpp
344

I didn't know this was an option! Thanks

aprantl added inline comments.Feb 5 2021, 3:24 PM
llvm/lib/IR/DebugInfo.cpp
344

This is weird — the code right below is already supposed to do this and I didn't notice!

aprantl added inline comments.Feb 5 2021, 3:26 PM
llvm/lib/IR/DebugInfo.cpp
344

Because it only looks at TermInsts.

aprantl updated this revision to Diff 321889.Feb 5 2021, 3:32 PM

Address review feedback!

Lesson learned — we can't depend on the IR being well-formed in the code that strips malformed annotations ;-)

My assumption is that the annotation shouldn't have been on the call to begin with, but I have bitcode produced by Apple clang-1100 from which this testcase was reduced.

vsk accepted this revision.Feb 5 2021, 4:33 PM

Neat, looks good to me!

This revision is now accepted and ready to land.Feb 5 2021, 4:33 PM