[llvm-commits] [llvm] r160148 - in /llvm/trunk: lib/CodeGen/AsmPrinter/DwarfDebug.cpp test/DebugInfo/X86/pr13303.ll

[David Blaikie]

On Mon, Sep 10, 2012 at 4:38 PM, Eric Christopher <echristo at apple.com> wrote:
>>
>> I finally got a round tuit... Ran gdb 7.2 test suite on a relatively recent TOT, and again with
>> this patch taken out.  Losing the patch makes the pass count go from 14000 to 14295.
>> The fail count also goes up a bit, from 901 to 916.  Hmmm...
>>
>> Looking at the differences more closely, I see that gbd.base/macscp.exp is the big fish.
>> With 160148, gdb is able to figure out that Clang doesn't generate macro info and skips this.
>> Without 160148, it can't figure that out, and it actually tries to run this test, with 300+ test points.
>> Excluding macscp.exp from the totals, I see:
>> with 160148: pass 13999, fail 901
>> without 160148: pass 14118, fail 776.
>>
>> Most of the remaining differences are because with 160148, setting a breakpoint on a
>> function name causes it to break on the first instruction, prior to the prolog.  This has
>> consequential effects, e.g. parameter values tend to be reported incorrectly, and also
>> the breakpoint tends to be outside the artificial lexical block wrapped around locals.
>>
>> The patch _does_ make the "list" command work better (!). But I think that's not
>> worth all the other trouble it causes.
>>
>> So, it looks to me like this patch should be reverted, and PR13303 will have to be solved
>> some other way. (Although it might be worth trying a more recent gdb test suite first.)
>>
>> Sorry about that, chief!
>
> No worries, I still think it might be partially correct, but that there are other underlying issues that need
> to be solved first. I've gone ahead and reverted it for now.

Just to continue this story, reapplied in 169304 after investigation
with the GDB 7.5 test suite showed this to be a fairly
sound/beneficial change. Open to further analysis/evidence, though.

- David