[llvm] r180140 - Make sure the instruction right after an inlined function has a

[Manman Ren]

On Apr 23, 2013, at 7:17 PM, Adrian Prantl wrote:

> 
> On Apr 23, 2013, at 3:44 PM, Eric Christopher <echristo at gmail.com> wrote:
> 
>> On Tue, Apr 23, 2013 at 8:56 PM, Adrian Prantl <aprantl at apple.com> wrote:
>>> Author: adrian
>>> Date: Tue Apr 23 14:56:03 2013
>>> New Revision: 180140
>>> 
>>> URL: http://llvm.org/viewvc/llvm-project?rev=180140&view=rev
>>> Log:
>>> Make sure the instruction right after an inlined function has a
>>> debug location. This solves a problem where range of an inlined
>>> subroutine is emitted wrongly.
>>> Patch by Manman Ren.
>>> 
>> 
>> Manman has commit rights. Why is she not committing this?
> I inherited the radar from her, she already had this patch, but it was depending on some other issue to be fixed first. So when that issue was fixed I went ahead and committed it, without taking credit for it, but with taking full responsibility for it :-)

Adrian,

Thanks for taking responsibility for it :)
 The problem was reported on a larger testing case and I came up with this small testing case to verify the patch.
Can you check whether the patch has an impact on the actual testing case?
I think I checked it before, but not so sure.

Thanks,
Manman

> 
>> I'm also not sure how this testcase was generated, it's not obvious
>> from an O0 compile of the sources you give in the testcase.
>> 
>> An O0 compile + opt -inline gives no diff if I do that.
> 
> Good catch. The problem is that even though the change is in llvm, the inlining happens in the frontend. Repaired by removing the always_inline attribute in the source code.
> 
> I’ve checked in the updated test (also addressing dblaikie’s comments) in r180168.
> 
>> The change from NewBr to CreatedBranchToNormalDest looks to be a
>> simple rename?
> Sort of. If we keep NewBr alive outside of that scope it ought to have a more specific name.
> 
>> There's also another set of branches created later that
>> don't have debug information put on them. This seems odd. opt -O2
>> seems to do the right thing, so I'm not sure.
> I’m unsure about this myself.
> 
>> As far as the testcase it does need attributes etc cleaned up.
> done.
>> Can you
>> show how the ranges are changed with this, i.e. we have plenty of
>> unconditional branches without line information - how is this
>> affecting the ranges differently? :)
> By ranges, do you mean the DW_AT_low/high_pc of the inlined subroutines in the dwarf? The patch fixes a disagreement between the range occupied by an inlined_subroutine and the line table. The high_pc would end up too low, because the branch out of the inlined_subroutine would not have any line info, and thus be counted towards the surrounding scope.
> 
> thanks,
> Adrian
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits