Improve inlined line numbers #14949

carnaval · 2016-02-05T20:22:13Z

start to remove filenames from line number nodes
get closer to remove the "old" Expr(:line, ...) form
improve dwarf generation for inline function line numbers
(get closer to what clang does)

I didn't check for regressions and sysimg size yet. Also the patch needs the frontend improvements from Jeff to remove filenames from the AST entierely (according to him, should be done today).

Probably not working on anything else than LLVM 3.7, I don't feel like fighting my way through the ifdef jungle just yet. Shouldn't we delete everything older than 3.7 now anyway ?

vtjnash · 2016-02-05T20:30:08Z

Shouldn't we delete everything older than 3.7 now anyway ?

not until it is equally performant. otherwise, how would you know when you succeed?

carnaval · 2016-02-08T21:54:35Z

It's starting to look ok. It handles arbitrary nested inlined functions, output the correct lambda infos and spec types.

The way it does it is kind of hacky but LLVM's dwarf reader does not make it possible to easily access custom argument so we store some things in the inlined function name.

The only case that does not work is when we do single expression inlining, we'll wait for the linear IR which solves it by definition.

vtjnash · 2016-02-28T05:17:19Z

what's the status of this / what does it still need to be merged?

ViralBShah · 2016-02-28T06:24:28Z

We could delete everything but LLVM 3.3 and 3.7 support.

carnaval · 2016-03-17T21:13:03Z

@Keno alright, it's rebased on top of your cleanup of those horrible files (that was painful, but I agree it's nicer now)

It's still missing a couple things :

put back support for file name changing in the middle of a function because of macros (the inlining case works of course since that was the whole point)
fix the build for llvm < 3.7
use the constant table index of the lambda instead of decompressing the ast for the lookup

The dwarf structure should look ok though, but I only looked at the llvm DI side of things, not at dwarfdump yet. Please do check that it behaves better if you're in the process of parsing the dwarf for the debugger anyway :-)

Keno · 2016-03-17T21:18:50Z

src/disasm.cpp

@@ -240,7 +240,7 @@ static int OpInfoLookup(void *DisInfo, uint64_t PC, uint64_t Offset, uint64_t Si
    size_t inlinedat_line;
    jl_lambda_info_t *outer_linfo;
    int fromC;
-    jl_getFunctionInfo(&name, &filename, &line, &inlinedat_file, &inlinedat_line, &outer_linfo, pointer, &fromC, skipC, 1);
+    //jl_getFunctionInfo(&name, &filename, &line, &inlinedat_file, &inlinedat_line, &outer_linfo, pointer, &fromC, skipC, 1);


Keno · 2016-03-18T04:04:09Z

Whitespace in source file is killing CI.

JeffBezanson · 2016-03-21T18:14:48Z

Could these be called push_loc and pop_loc instead of push_lambda? For macro locations we would need to put in a file name instead of a LambdaInfo.

JeffBezanson · 2016-03-21T18:38:59Z

Also, please merge as soon as you can and I'll fix macro line numbers after.

carnaval · 2016-03-25T18:55:42Z

alright this is now on top of jb/linear3 and uses a field in the linfo as we discussed.

still need to fix llvm 3.3 build I think, let's first see what CI says about that

carnaval · 2016-03-25T18:57:20Z

(teaser

ERROR: LoadError: hi
 in f(::Int64, ::Bool) at /home/carnaval/dev/julia/repro2.jl:5 [inlined]
 in g(::Int64, ::Bool) at /home/carnaval/dev/julia/repro2.jl:13 [inlined]
 in h(::Bool) at /home/carnaval/dev/julia/repro2.jl:19

with all three frames from the same physical function)

yuyichao · 2016-03-25T19:09:50Z

(

hi

Oscar's favorate English word =D )

This is awesome!

vtjnash · 2016-03-26T02:59:54Z

fixes #13725

Keno · 2016-03-29T07:00:29Z

@JeffBezanson @carnaval Can we get this merged soon?

carnaval · 2016-03-29T12:30:35Z

well it needs jb/linear3 to be merged first, and it's also broken unfortunately. Apart from at least a bug, keeping all those lambdas around is also putting quite a strain on the serializer apparently :/ I'm having a look.

JeffBezanson · 2016-03-29T18:52:27Z

Ok, let's merge jb/linear3 today.

JeffBezanson · 2016-03-29T21:45:15Z

Ok, done.

JeffBezanson · 2016-03-30T23:47:19Z

Any update here?

vtjnash · 2016-04-04T02:52:29Z

we're retiring workers much faster, so it appears that memory usage is a fair bit higher. with the latest change, it looks like we are just barely able to sneak this through CI, but two of the tests need to be updated for the changes.

tkelman · 2016-04-04T18:28:28Z

Note that the 32 bit linux buildbot has been running out of memory for several days, preventing the build of (gallium-compatible) nightlies. Would like to avoid making it any worse.

Will also need some squashing cleanup.

carnaval · 2016-04-04T18:31:06Z

yeah all those temporary commits are just to try and see if the mem usage actually improves as I fix things, will squash.

I'm also planning to investigate more closely why, for example, linalg/triangular needs 1GB of resident memory (even on master) and I don't think generated code is the only culprit. We'll see.

vtjnash · 2016-04-04T18:45:18Z

Note that the 32 bit linux buildbot has been running out of memory for several days, preventing the build of (gallium-compatible) nightlies. Would like to avoid making it any worse.

would be good to open these observations as issues (most of us don't watch the buildbots too closely). It looks like the last successful build was the merge commit for #15609. the next commits were trivial cleanup, so I'm thinking jb/linear3 seems to be the smoking gun here?

tkelman · 2016-04-04T20:20:57Z

Yes I think that's right. There was one later successful build which threw me off.

tkelman · 2016-04-06T17:32:10Z

I managed to fix the issue for now by rebooting that specific build worker. I think what happened is extra julia processes were accumulating and not getting killed properly when jobs timed out. Don't know whose bug that is, but it's okay for now.

vtjnash · 2016-04-15T17:57:32Z

src/debuginfo.cpp

+#ifdef _OS_DARWIN_
+#define MAP_ANONYMOUS MAP_ANON
+#endif
+uint8_t *juliaAllocateSection(uintptr_t size, unsigned align, section_type typ)


this seems unrelated to the PR, can it be pulled out into a separate commit?

carnaval force-pushed the ob/dwarf-inline branch from eed214a to abf7d5e Compare February 8, 2016 21:52

vtjnash mentioned this pull request Mar 17, 2016

Our DWARF info makes no sense #15537

Closed

carnaval force-pushed the ob/dwarf-inline branch from abf7d5e to 62b54f6 Compare March 17, 2016 21:08

Keno reviewed Mar 17, 2016
View reviewed changes

JeffBezanson mentioned this pull request Mar 22, 2016

WIP: overhaul file name info #15583

Closed

4 tasks

carnaval force-pushed the ob/dwarf-inline branch 3 times, most recently from 936bd32 to b1c9f7d Compare March 25, 2016 18:53

carnaval force-pushed the ob/dwarf-inline branch 2 times, most recently from 4f31caa to 9ab0f11 Compare March 25, 2016 19:22

Keno mentioned this pull request Mar 27, 2016

Release Blocking Issues JuliaDebug/Gallium.jl#48

Open

8 tasks

yuyichao mentioned this pull request Mar 29, 2016

Handle compressed AST in jl_local_in_linfo #15672

Closed

carnaval force-pushed the ob/dwarf-inline branch from 9ab0f11 to 7788363 Compare April 1, 2016 17:59

carnaval force-pushed the ob/dwarf-inline branch from a5ecd47 to a3dcb8f Compare April 11, 2016 17:21

JeffBezanson mentioned this pull request Apr 12, 2016

compiler performance #14743

Closed

carnaval force-pushed the ob/dwarf-inline branch 4 times, most recently from f90e28d to ea72622 Compare April 15, 2016 10:43

Improve inlined line numbers

bff74b1

carnaval force-pushed the ob/dwarf-inline branch from ea72622 to bff74b1 Compare April 15, 2016 11:50

vtjnash reviewed Apr 15, 2016
View reviewed changes

carnaval added 2 commits April 18, 2016 15:22

wip

ed133c7

get stacktraces from macro expansion errors

e3993d3

vtjnash mentioned this pull request Apr 27, 2016

Error messages, stack traces, Line Numbers... #10595

Closed

JeffBezanson mentioned this pull request May 12, 2016

RFC: improve inlined line numbers, take 2 #16335

Merged

JeffBezanson closed this Jun 28, 2016

tkelman deleted the ob/dwarf-inline branch June 29, 2016 01:42

vtjnash mentioned this pull request Jul 20, 2016

inlined code line numbers need work #13725

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve inlined line numbers #14949

Improve inlined line numbers #14949

carnaval commented Feb 5, 2016

vtjnash commented Feb 5, 2016

carnaval commented Feb 8, 2016

vtjnash commented Feb 28, 2016

ViralBShah commented Feb 28, 2016

carnaval commented Mar 17, 2016

Keno Mar 17, 2016

Keno commented Mar 18, 2016

JeffBezanson commented Mar 21, 2016

JeffBezanson commented Mar 21, 2016

carnaval commented Mar 25, 2016

carnaval commented Mar 25, 2016

yuyichao commented Mar 25, 2016

vtjnash commented Mar 26, 2016

Keno commented Mar 29, 2016

carnaval commented Mar 29, 2016

JeffBezanson commented Mar 29, 2016

JeffBezanson commented Mar 29, 2016

JeffBezanson commented Mar 30, 2016

vtjnash commented Apr 4, 2016

tkelman commented Apr 4, 2016

carnaval commented Apr 4, 2016

vtjnash commented Apr 4, 2016

tkelman commented Apr 4, 2016

tkelman commented Apr 6, 2016

vtjnash Apr 15, 2016

Improve inlined line numbers #14949

Improve inlined line numbers #14949

Conversation

carnaval commented Feb 5, 2016

vtjnash commented Feb 5, 2016

carnaval commented Feb 8, 2016

vtjnash commented Feb 28, 2016

ViralBShah commented Feb 28, 2016

carnaval commented Mar 17, 2016

Keno Mar 17, 2016

Choose a reason for hiding this comment

Keno commented Mar 18, 2016

JeffBezanson commented Mar 21, 2016

JeffBezanson commented Mar 21, 2016

carnaval commented Mar 25, 2016

carnaval commented Mar 25, 2016

yuyichao commented Mar 25, 2016

vtjnash commented Mar 26, 2016

Keno commented Mar 29, 2016

carnaval commented Mar 29, 2016

JeffBezanson commented Mar 29, 2016

JeffBezanson commented Mar 29, 2016

JeffBezanson commented Mar 30, 2016

vtjnash commented Apr 4, 2016

tkelman commented Apr 4, 2016

carnaval commented Apr 4, 2016

vtjnash commented Apr 4, 2016

tkelman commented Apr 4, 2016

tkelman commented Apr 6, 2016

vtjnash Apr 15, 2016

Choose a reason for hiding this comment