[PATCH] Debug Info Verifier pass

[Manman Ren]

On Mon, Apr 14, 2014 at 6:36 PM, Manman Ren <manman.ren at gmail.com> wrote:

>
>
>
> On Mon, Apr 14, 2014 at 5:56 PM, Duncan P. N. Exon Smith <
> dexonsmith at apple.com> wrote:
>
>>
>> On Apr 11, 2014, at 11:31 AM, Manman Ren <manman.ren at gmail.com> wrote:
>>
>> > Hi Duncan,
>> >
>> > Thanks for working on this. The patches look great overall.
>> > 0001: LGTM
>> > 0002: There is some inconsistency in creating debug info verifier pass.
>> > 1>
>> > tools/opt/opt.cpp:  if (VerifyEach) PM.add(createVerifierPass());
>> > Should VerifyEach guard debug info verifier as well given that we are
>> using NoVerify to guard debug info verifier?
>> > I am okay with not using VerifyEach for debug info verifier, but please
>> add comments there to explain why.
>>
>> I just changed it to line up with Verifier for now.  Probably it will
>> be too slow, but if it is, we'll probably want to add a separate flag
>> so we can --verify-di-each on demand.
>>
>> > 2> What about these two places? Should we add debug info verifier?
>> > tools/clang/lib/CodeGen/BackendUtil.cpp:
>>  FPM->add(createVerifierPass());
>> > unittests/Bitcode/BitReaderTest.cpp:  passes.add(createVerifierPass());
>>
>> I just missed those.  Thanks!  Note that one of those is in cfe, so
>> I've added a third patch for that one.
>>
>> > 3> Could you turn debug info verifier on by default and make sure it is
>> working correctly with "make check-all"?
>>
>> I'm seeing 225 failures with make check-all when I turn --verify-di
>> and --verify-each on by default (configured with X86;ARM;ARM64):
>>
>>   Expected Passes    : 14993
>>   Expected Failures  : 58
>>   Unsupported Tests  : 2341
>>   Unexpected Failures: 225
>>
>> make check gives:
>>
>>   Expected Passes    : 7893
>>   Expected Failures  : 39
>>   Unsupported Tests  : 2288
>>   Unexpected Failures: 225
>>
>> so they're all in LLVM proper.
>>
>> Is this expected?  Given that I know next to nothing about debug
>> info, I'm probably not the right person to diagnose them.
>>
>
> These tests may fail as well with the current debug info verifier when we
> turn it on by default (without your patch).
>
>
>>
>> Should they be fixed before commit?  I'm thinking it would be
>> better to tackle them incrementally.
>>
>
> I am okay with committing your patches as long as we see the same set of
> failures without your patch.
> We should file a bug to track these failing tests though.
>

Since Eric is not worried about these failing cases, these patches LGTM.

Manman


>
> Thanks,
> Manman
>
>
>> (For clarity, in case someone else on the list isn't following
>> closely, check-all is clean unless I *force* these options on.)
>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140414/6f2a3e3b/attachment.html>