[PATCH] D18518: Add an IR Verifier check for orphaned DICompileUnits.

Duncan P. N. Exon Smith via llvm-commits llvm-commits at lists.llvm.org
Mon Mar 28 14:04:40 PDT 2016


> On 2016-Mar-28, at 13:57, Adrian Prantl <aprantl at apple.com> wrote:
> 
> aprantl updated this revision to Diff 51836.
> aprantl added a comment.
> 
> Address review comments.
> 
> 
> http://reviews.llvm.org/D18518
> 
> Files:
>  lib/IR/Verifier.cpp
>  test/Assembler/dicompileunit.ll
>  test/Transforms/LoopIdiom/debug-line.ll
>  test/Verifier/dbg-orphaned-compileunit.ll
>  tools/bugpoint/CrashDebugger.cpp
> 
> <D18518.51836.patch>

LGTM.


> On 2016-Mar-28, at 12:38, Adrian Prantl <aprantl at apple.com> wrote:
> 
>> 
>> On Mar 28, 2016, at 11:40 AM, Duncan P. N. Exon Smith <dexonsmith at apple.com> wrote:
>> 
>> How does this interact with Keno's changes to make bugpoint reduce debug
>> info?  Will the compile units still get pruned?
> 
> Yes, there is a testcase BugPoint/named-mds.ll that verifies that DICompileUnits are still pruned. It’s just the named llvm.dbg.cu node that will no longer get removed.
> I shall add a check for an empty CU list, which should be safe to remove.

Great.

>> It seems more efficient to do this directly, rather than creating a
>> third new set and checking that it's empty:
>> 
>>   Assert(CUVisited.size() <= Listed.size());
>>   Assert(std::all_of(CUVisited.begin(), CUVisited.end(),
>>              [&Listed](const Metadata *CU) { return Listed.count(CU); });
>> 
> 
> Thanks!
> 
>> Maybe it's worth adding this to SetOperations.h (I'm not sure), but I
>> think keeping the Verifier efficient is important.
> 
> I don’t know what a good name for such a function would be.
> 

Neither to I.


More information about the llvm-commits mailing list