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

[Eric Christopher]

>> 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 :-)
>

OK. Add some comments then...

>> 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.
>

OK. It's much easier to see the change in behavior here. Thanks.

>> 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.
>

OK. You should probably investigate.

>> 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.
>

Yes, I mean that exactly. The problem is that as far as I can tell
your patch doesn't end up changing the behavior there?

dhcp-172-16-23-31:~/tmp> ~/builds/build-llvm/Debug+Asserts/bin/opt
-inline -S -o - foo.ll | ~/builds/build-llvm/Debug+Asserts/bin/llc
-filetype=obj -o old.o -O0

<apply patch>

dhcp-172-16-23-31:~/tmp> ~/builds/build-llvm/Debug+Asserts/bin/opt
-inline -S -o - foo.ll | ~/builds/build-llvm/Debug+Asserts/bin/llc
-filetype=obj -o new.o -O0
dhcp-172-16-23-31:~/tmp>
~/builds/build-llvm/Debug+Asserts/bin/llvm-dwarfdump new.o > new.out
dhcp-172-16-23-31:~/tmp>
~/builds/build-llvm/Debug+Asserts/bin/llvm-dwarfdump old.o > old.out
dhcp-172-16-23-31:~/tmp> diff old.out new.out

    1c1
< old.o: file format Mach-O 64-bit x86-64
---
> new.o: file format Mach-O 64-bit x86-64

Can you duplicate the behavior you're expecting with your testcase and
show me how you did it?

-eric