Re: Mirage/kFreeBSD Compiler Patches

[PALI Gabor Janos <pgj@xxxxxxx>]

On Thu, Sep 05, 2013 at 11:59:11AM +0100, Anil Madhavapeddy wrote:
> Thanks for breaking these up Gabor!

Thanks for the comments.

> I expect that a number of the patches
> are generally relevant to other backends in the future, as we move to 
> establish
> the differences between many of the backends.

Yes, that is likely.  However, let me note that I have some further minor
modifications in the run-time system as well.  I did not include them here 
though,
because they are currently part of the mirage-kfreebsd package -- due to the
previously mentioned API compatibility, they work with the compiler-generated
code fine, but care should be taken when moving to a newer version of the
run-time bits.

For example, I have recently been bitten by a one-line change which works fine
in user space but the kernel (asmrun/amd64.S):

/* Call a C function from OCaml */

FUNCTION(G(caml_c_call))
CFI_STARTPROC
LBL(caml_c_call):
    /* Record lowest stack address and return address */
        popq    %r12; CFI_ADJUST(-8)
        STORE_VAR(%r12, caml_last_return_address)
        STORE_VAR(%rsp, caml_bottom_of_stack)
        subq    $8, %rsp; CFI_ADJUST(8) /* equivalent to pushq %r12 */
...

Actually, `pushq %r12` cannot be replaced with `subq $8, %rsp` here as the
assumption in the comment does not hold inside the kernel.  This is because the
standard AMD64 ABI for C calls (use of red zones) is not respected in the
FreeBSD kernel (neither in Linux too, I think).

> > Patch: 
> > https://github.com/pgj/ocaml/commit/e63fdcbad6b403a52a7458e849136b08f97bb3d3.patch
> > Synopsis: Add support for explicitly disabling PIC code generation to
> >          OCAMLPARAM.
[..]
> It's probably best to disable this by default in ARM.  How about modifying
> the Clflags to be either `True|`False|`Default, and then using that to decide
> what the architecture-specific backend should be?
> 
> I expect that PIC code is a lot more expensive on ARM than it is on AMD64,
> although it's worth verifying this with an upstream patch submission.

Yes, PIC is turned off by default on ARM.  Hence I extended this patch with
another one to retain the original semantics:

https://github.com/pgj/ocaml/commit/acb827c29187de3fafd17561e6e53606de33a94a.patch

> > Patch: 
> > https://github.com/pgj/ocaml/commit/2e155b0c4e551f3cfb31aedbe44b4304ad2e4ccf.patch
> > Synopis: Allow extra libraries to be cleaned/installed separately.
[..]
> This one probably won't get merged, since 4.02 is moving out all the otherlibs
> into separate packages that are outside the core distro.  This is good news 
> for
> your port anyway!  I'll dig out the Mantis upstream issue for this.

Excellent.

> > Patch: 
> > https://github.com/pgj/ocaml/commit/26196344b32a1bb243b0e8986e5c0c82b05cc262.patch
> > Synopsis: Add fixed-point arithmetic support for doubles, unroll
> >          floating-point primops.
[..]
> Looks good -- I agree with your assessment that this isn't mergeable until
> we precisely define what sort of impact this will have, but the strategy of
> "carefully avoid floating point or else crash" seems to be working well enough
> for now.

If you think it may be worth to add this to upstream, I could implement a
dedicated flag, something like `-ffixedpt=N`.  And depending on its value, the
native code generator could emit inline primops for the fixed-point arithmetic
to avoid the C function calls, while the non-primitive functions could be
incorporated in the run-time system as well.

> Do you want to submit the nopic one upstream for now and start some discussion
> going on that?

Sure.   What about submitting

https://github.com/pgj/ocaml/commit/acb827c29187de3fafd17561e6e53606de33a94a.patch
https://github.com/pgj/ocaml/commit/e63fdcbad6b403a52a7458e849136b08f97bb3d3.patch

as a single patch?