[bytecode runtime] Rework fixed register assignments on m68k #13252
Conversation
ghost
mentioned this pull request
|
This PR appears to fix a bug that would let Debian testing progress further, and in this sense it is good. On the other hand I am not sure about the idea of removing the register assignment because there are too few registers. Even when there are few, quite probably the PC, SP and ACCU registers of the VM are those you really want to have in machine registers. Has this calculus changed in OCaml 5 rather than OCaml 4? Could you suggest other registers to reserve instead? (Do we even have a reliable way to do a performance comparison of the two approaches?) |
This has not changed from OCaml 4 and I expect OCaml 4 to fail the same way - at least when the interpreter needs to invoke a libc function which accesses global data (and thus will perform On m68k, register allocation is painful, because although there are 16 general-purpose registers, they are not all equal, only It might be worth trying to keep fixed allocations but use |
|
I would be more at ease with the minimal/conservative change (using a3 instead of a5) first. We could wait for a performance test (which probably is never going to come because few people care about this architecture) to remove the assignments altogether. |
How about this new version? |
ghost
changed the title
|
I feel happy with this new version -- as in, I don't have the expertise to ensure that this is right, but my risk assessment is that this is very probably fine if you say it is, so we could go ahead. What are the benefits of keeping a5 on non-linux systems, instead of switching to a3 uniformly, which seems simpler? @glondu if you have an easy way to check that this patch works just as well as disabling the register assignment, I would be interested in a confirmation. But I am thinking of approving&merging it anyway. |
I don't know. If someone is willing to dig into history, the rationale behind the choice of the existing assignment might provide useful information (as in: "we tried various combinations and this one turns out to be better because...") but I'm not sure there are traces of it. |
gasche
left a comment
gasche
left a comment
There was a problem hiding this comment.
Hm. I don't understand enough about the domain to see why a5->a3 would make a difference, but I don't want to insist and I am happy to approve the PR as-is.
|
I looked at the git history, the m68k register assignment was chosen in 1996 by @xavierleroy and never changed since: 9a374eb. |
|
Yet this proves there were still Sun3 running in INRIA in 1996! At least in Rocquencourt - INRIA Sophia-Antipolis was already full SPARC (and Linux) at that time. 😀 |
I've just uploaded to experimental a new version of the package, with the current state of this PR + disabling runtime_events on armel. Results should appear in a few hours there: https://buildd.debian.org/status/package.php?p=ocaml&suite=experimental |
xavierleroy
left a comment
xavierleroy
left a comment
There was a problem hiding this comment.
So there's still one M68k machine compiling Debian packages... (I doubt anyone uses those packages, esp. the OCaml ones, but that's the way it is). Proposed simplification below.
I probably restarted an old Sun 3/60 that was lying around just to do this M68k port. In retrospect, I should not have. |
The use of %a5 conflicts with its internal usage as a GOT base by Glibc, so shift assignments one register down.
ghost
deleted the
The use of %a5 conflicts with its internal usage as a GOT base by Glibc, so shift assignments one register down. Origin: ocaml#13252
The use of %a5 conflicts with its internal usage as a GOT base by Glibc, so shift assignments one register down. Origin: ocaml/ocaml#13252 Gbp-Pq: Name 0009-Rework-fixed-register-assignments-on-m68k.patch
The use of %a5 conflicts with its internal usage as a GOT base by Glibc, so shift assignments one register down. Origin: ocaml/ocaml#13252 Gbp-Pq: Name 0009-Rework-fixed-register-assignments-on-m68k.patch
The use of %a5 conflicts with its internal usage as a GOT base by Glibc, so shift assignments one register down. Origin: ocaml/ocaml#13252 Gbp-Pq: Name 0009-Rework-fixed-register-assignments-on-m68k.patch
4 participants
As reported in #13249, the fixed use of %a5 in the bytecode interpreter conflicts with its internal usage as a GOT base by Glibc, and given the scarce number of available registers on this platform, it is probably better to trust the compiler to perform register assignment here.