Do not emit references to dead labels (spacetime) by gretay-js · Pull Request #9097 · ocaml/ocaml

gretay-js · 2019-11-06T15:02:37Z

Dead code elimination in the backend can remove spacetime instrumentation and labels that spacetime table refers to. The table emitted at the end of a compilation unit still contains references to these now-undefined labels, causing a linker error. This PR fix it.

For example, the code

type op = Eq | Neq
let [@inline always] f a = print_endline (string_of_int a)
let foo op =
  let a = true in
  match op, a with
  | Eq, true -> f 1
  | Neq, true -> f 2
  | _, _ -> f 3

fails with a linker error:

$ ocamlopt test2.ml -o test 
test2.o: In function `camlTest2__spacetime_shapes':
:(.data+0x190): undefined reference to `.L112'
:(.data+0x1a8): undefined reference to `.L111'
collect2: error: ld returned 1 exit status
File "caml_startup", line 1:
Error: Error during linking

when compiled with spacetime and without flambda.

With flambda, the match is simplified before spacetime table is computed. Without flambda, the calls are instrumented with spacetime and f is inlined. Then, dead code pass in the backend eliminates some of the inlined calls to f, including instrumentation, but spacetime table is not updated.

This is most likely caused by PR #2321 (in 4.10) that eliminates dead handlers, which may include inlined calls, and hence instrumentation and labels. It seems that prior to it, labels have never been eliminated in the backend.

The proposed fix is to keep track of local labels that are emitted for a compilation unit, and then use this information to clean spacetime table right before it is emitted. The fix is in amd64/emit.mlp because spacetime is specific to amd64 and dead code elimination pass is generic.

Keeping track of emitted labels has some overhead in terms of compilation time and memory, but it is only enabled when the compiler is configured with spacetime. Note that used_labels is a superset of labels that spacetime can refer to, but it simplifies the code that keeps track of them.

Testsuite: two tests are currently failing in my setup with spacetime enabled. The failures are unrelated to the PR: they fail without this patch.

statmemprof/native does not terminate
output-complete-obj/test fails to find libunwind.

All other tests pass. The example above compiles and works under spacetime.

gasche · 2019-11-07T10:57:11Z

Meta-level note: I wonder how important it is to work on fixing spacetime instrumentation in trunk. Last I heard, instrumentation tools where only available for an older version of OCaml, so people would basically use those versions to profile their programs. Are there active users of Spacetime on recent OCaml versions in the wild? If you do not presently have an active need for Spacetime, maybe it's reasonable to invest less resources into fixing it right now.

mshinwell · 2019-11-07T12:21:02Z

@gasche We're using Spacetime to profile Flambda 2.0, so "yes" :) The existing tools work just fine.

mshinwell

This is ok for merging after comments addressed.

asmcomp/amd64/emit.mlp

gretay-js · 2019-12-10T12:25:05Z

This should probably be backported to 4.10. The issue it fixes was introduced in PR #2321 which is included in 4.10.

gasche · 2019-12-10T12:29:39Z

(Backporting sounds sensible to me, but I don't have time to do it now. Whoever should feel free to do it, please use git cherry-pick -x -m 1 <github merge commit>.)

ocaml#8938 from upstream trunk (cherry-pick 4353c75) ocaml#9090 from upstream trunk (cherry-pick fe7c8ed) ocaml#9097 from upstream trunk (cherry-pick 6daaf62) ocaml#8939 from gretay-js/save-linear (cherry-pick 1200de9) ocaml#9003 from gretay-js/start-from-emit (cherry-pick 21d5d26)

ocaml#8938 from upstream trunk (cherry-pick 4353c75) ocaml#9090 from upstream trunk (cherry-pick fe7c8ed) ocaml#9097 from upstream trunk (cherry-pick 6daaf62) ocaml#8939 from gretay-js/save-linear (cherry-pick 1200de9) ocaml#9003 from gretay-js/start-from-emit (cherry-pick 21d5d26) Add linear_format to dune file

ocaml#8938 from upstream trunk (cherry-pick 4353c75) ocaml#9090 from upstream trunk (cherry-pick fe7c8ed) ocaml#9097 from upstream trunk (cherry-pick 6daaf62) ocaml#8939 from gretay-js/save-linear (cherry-pick 1200de9) ocaml#9003 from gretay-js/start-from-emit (cherry-pick 21d5d26) Add linear_format to dune file Update .depend

gretay-js force-pushed the fix_dce_spacetime_labels branch from 248c965 to 3f4c437 Compare November 6, 2019 16:31

gretay-js mentioned this pull request Nov 7, 2019

Fix libunwind flags (spacetime) #9099

Closed

mshinwell requested changes Nov 20, 2019

View reviewed changes

asmcomp/amd64/emit.mlp Outdated Show resolved Hide resolved

asmcomp/amd64/emit.mlp Outdated Show resolved Hide resolved

gretay-js added 3 commits November 20, 2019 18:23

Do not emit references to dead labels (spacetime)

2839f73

Address review comments

b98ad1d

Add Changes entry

50c76ed

gretay-js force-pushed the fix_dce_spacetime_labels branch from 67e84d0 to 50c76ed Compare November 20, 2019 18:26

mshinwell approved these changes Nov 26, 2019

View reviewed changes

mshinwell merged commit 6daaf62 into ocaml:trunk Nov 26, 2019

stedolan added a commit to janestreet/ocaml that referenced this pull request Mar 17, 2020

ocaml#9097 from upstream trunk (cherry-pick 6daaf62)

ca743fc

gretay-js added a commit to gretay-js/ocaml that referenced this pull request Jun 4, 2020

ocaml#9097 from upstream trunk (cherry-pick 6daaf62)

db2cf31

gretay-js mentioned this pull request Nov 19, 2020

Spacetime (4.10.1): "error: assembler local symbol [...] not defined" #10030

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Do not emit references to dead labels (spacetime)#9097

Do not emit references to dead labels (spacetime)#9097
mshinwell merged 3 commits intoocaml:trunkfrom
gretay-js:fix_dce_spacetime_labels

gretay-js commented Nov 6, 2019

Uh oh!

gasche commented Nov 7, 2019

Uh oh!

mshinwell commented Nov 7, 2019

Uh oh!

mshinwell left a comment

Uh oh!

Uh oh!

Uh oh!

gretay-js commented Dec 10, 2019 •

edited

Loading

Uh oh!

gasche commented Dec 10, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

gretay-js commented Nov 6, 2019

Uh oh!

gasche commented Nov 7, 2019

Uh oh!

mshinwell commented Nov 7, 2019

Uh oh!

mshinwell left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

gretay-js commented Dec 10, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gasche commented Dec 10, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

gretay-js commented Dec 10, 2019 •

edited

Loading