[PATCH] D21818: Add artificial debug information to avoid compiler crash

[Gao, Yunzhong via llvm-commits]

Hi Adrian,
I did the following experiment.
Step#1, check out from trunk r279786
Step#2, revert the following commits: r267370, r267904 and r274263
Step#3, build and run regression test => all tests pass with no failure

In addition,
I tested the compiler built above with this test added in r267370, llvm/test/Verifier/callsite-dbgloc.ll,
the compilation succeeds with no assertion failure. This test has a comment at the top of the file
which mentions a C example that was expected to get an assertion failure with "-O2 -g", but upon
my experiment, no assertion failure was triggered either.

I tested the compiler built above with the test change introduced in r274263, and again, the test
passed with no assertion failure.

What do you think of this experiment? Can you draw meaningful conclusions from this,
or am I missing some steps?

- Gao



________________________________________
From: aprantl at apple.com [aprantl at apple.com]
Sent: Thursday, August 25, 2016 10:52 AM
To: Robinson, Paul
Cc: Gao, Yunzhong; David Blaikie; reviews+D21818+public+b40f0be86340e0e5 at reviews.llvm.org; llvm-commits at lists.llvm.org
Subject: Re: [PATCH] D21818: Add artificial debug information to avoid compiler crash

> On Aug 25, 2016, at 10:49 AM, Robinson, Paul <paul.robinson at sony.com> wrote:
>
> (re-adding llvm-commits which got lost from my previous)
>
>> -----Original Message-----
>> From: aprantl at apple.com [mailto:aprantl at apple.com]
>> Sent: Thursday, August 25, 2016 10:42 AM
>> To: Robinson, Paul
>> Cc: Gao, Yunzhong; David Blaikie;
>> reviews+D21818+public+b40f0be86340e0e5 at reviews.llvm.org
>> Subject: Re: [PATCH] D21818: Add artificial debug information to avoid
>> compiler crash
>>
>>
>>> On Aug 25, 2016, at 10:37 AM, Robinson, Paul <paul.robinson at sony.com>
>> wrote:
>>>> -----Original Message-----
>>>> From: llvm-commits [mailto:llvm-commits-bounces at lists.llvm.org] On
>> Behalf
>>>> Of Gao, Yunzhong via llvm-commits
>>>> Hi Adrian,
>>>> I would not say that attaching an artificial debug location should be
>>>> ruled out as
>>>> an option, and the original commit r274263 used that approach. But I
>> think
>>>> David
>>>> was suggesting that the verifier could be modified to consider this IR
>> as
>>>> valid
>>>> without the extra debug info (that was my interpretation of what David
>>>> said,
>>>> but I could be wrong) and therefore I did some investigation here,
>>>
>>> Right, the line-0 tactic increases debug-info size but the original
>>> complaint was because of a verifier failure.  If we can mark the
>>> offending artificial calls so that the verifier will allow them,
>>> then we don't pay the debug-info size cost in the object file.
>>>
>>> I'd be happier with a tactic that would automatically deal with this
>>> for all cases of artificial calls to hard-coded function names, but
>>> it's not immediately clear how to make that work.
>>
>> I added the verifier check because having an inlineable function call
>> without a debug location to a function with debug info inside a function
>> with debug info causes an assertion failure when the backend attempts to
>> construct the inline scopes.
>> Do we know that it is safe to relax the verifier here?
>>
>> -- adrian
>>
>>
> Well, maybe not, and that would be a good experiment for Gao to try--
> undo the original patch, so it doesn't mark the call with line-0, and
> make the called function inlineable, and see if it dies when generating
> the inlined scopes.

Gao, I think the assertion we're looking for is:

LexicalScopes::getOrCreateRegularScope()
  assert(cast<DISubprogram>(Scope)->describes(MF->getFunction()));

> --paulr
>