This is an archive of the discontinued LLVM Phabricator instance.

[mips] Fix 'jumpy' debug line info around calls.
ClosedPublic

Authored by dsanders on Jan 19 2015, 2:45 AM.

Details

Summary

At the moment, address calculation is taking the debug line info from the
address node (e.g. TargetGlobalAddress). When a function is called multiple
times, this results in output of the form:

.loc $first_call_location
.. address calculation ..
.. function call ..
.. address calculation ..
.loc $second_call_location
.. function call ..
.loc $first_call_location
.. address calculation ..
.loc $third_call_location
.. function call ..

This patch makes address calculations for function calls take the debug line
info for the call node and results in output of the form:

.loc $first_call_location
.. address calculation ..
.. function call ..
.loc $second_call_location
.. address calculation ..
.. function call ..
.loc $third_call_location
.. address calculation ..
.. function call ..

All other address calculations continue to use the address node.

Diff Detail

Event Timeline

dsanders updated this revision to Diff 18376.Jan 19 2015, 2:45 AM
dsanders retitled this revision from to [mips] Fix 'jumpy' debug line info around calls..
dsanders updated this object.
dsanders edited the test plan for this revision. (Show Details)
dsanders added a reviewer: vmedic.
dsanders added a subscriber: Unknown Object (MLST).

It'd be good to have an explicit test for this (in case the multiline test is ever refactored/removed to test in a different way) with an explicit mips triple (so that even people who's host architecture isn't mips will see this failure immediately if they regress this behavior).

dsanders updated this revision to Diff 18421.Jan 20 2015, 2:11 AM

Added explicit test case (based on test/Debug/multiline.ll) and added
comments explaining that it's intended to test that 'jumpy' line info is not
emitted.

Is this what you had in mind? I've essentially copied multiline.ll, added -mtriple=mips-linux-gnu, and explained the 'jumpy' line information problem.

Is this what you had in mind? I've essentially copied multiline.ll, added -mtriple=mips-linux-gnu, and explained the 'jumpy' line information problem.

More or less - I'd simplify it to only the basics needed (two function calls to the same function - I'd put them on different lines, because the test case isn't about interesting things that happen when they're on the same line, or when there's 3 of them, etc).

I might simplify the description a bit, but your description's fine too.

dsanders updated this revision to Diff 18515.Jan 21 2015, 7:28 AM

Reduced the testcase to the minimum required (two functions rather than six and
functions are on different lines).

dsanders updated this revision to Diff 18516.Jan 21 2015, 7:37 AM

Previous revision deleted a little too much and it started passing without the
fix. Brought back one more function call.

On Wed, Jan 21, 2015 at 7:37 AM, Daniel Sanders <daniel.sanders@imgtec.com<mailto:daniel.sanders@imgtec.com>> wrote:
Previous revision deleted a little too much and it started passing without the
fix. Brought back one more function call.

Why were 2 calls insufficient?

The first spurious '.loc 3' we are looking for is always omitted because the previous call is on line 3 too. With a third call, a second spurious '.loc 3' will appear between the second and third calls.

From: David Blaikie [mailto:dblaikie@gmail.com]
Sent: 21 January 2015 17:07
To: reviews+D7050+public+e22ff8356bd9a5ae@reviews.llvm.org
Cc: Daniel Sanders; Vladimir Medic; llvm-commits@cs.uiuc.edu; ehostunreach@gmail.com
Subject: Re: [PATCH] [mips] Fix 'jumpy' debug line info around calls.

dsanders updated this revision to Diff 18599.Jan 22 2015, 3:09 AM

Test the .loc positioning more directly by matching the %call16 reloc.

The extra nots/checks/suffixes don't seem important to the essence of this test.

I agree. I'll prune that in the commit.

I'll leave it to someone MIPS-y to sign off on the actual code change (unless you were only seeking review of the test case part of things & you're confident in the code change itself).

I'd normally wait for a full pre-commit sign-off regardless of my confidence level but it's a safe change and it should make the builder green again so I think it's best to make an exception.


From: David Blaikie [dblaikie@gmail.com]
Sent: 22 January 2015 17:47
To: reviews+D7050+public+e22ff8356bd9a5ae@reviews.llvm.org
Cc: Daniel Sanders; Vladimir Medic; llvm-commits@cs.uiuc.edu; ehostunreach
Subject: Re: [PATCH] [mips] Fix 'jumpy' debug line info around calls.

This revision was automatically updated to reflect the committed changes.