Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FFI aggregate handling for mips64 is broken #47290

Closed
jcowgill opened this issue Jan 9, 2018 · 1 comment
Closed

FFI aggregate handling for mips64 is broken #47290

jcowgill opened this issue Jan 9, 2018 · 1 comment
Labels
A-ffi Area: Foreign Function Interface (FFI) C-bug Category: This is a bug. O-MIPS Target: MIPS processors

Comments

@jcowgill
Copy link
Contributor

jcowgill commented Jan 9, 2018

A number of FFI tests relating to the handling of aggregates fail on 64-bit MIPS:
https://gist.github.com/jcowgill/911a7c3cf3c05c187c7ed1262526de78

It seems to me that src/librustc_trans/cabi_mips64.rs has been copied from cabi_mips.rs with only minimal changes, however the 64-bit MIPS ABI is significantly different from the 32-bit ABI so I expect that file needs rewriting.

I've had a brief look fixing this (no patches yet). One issue is that the 64-bit MIPS ABI requires special handling of any structures containing doubles, not just homogeneous structures. This means I can't cast these to Uniform like other architectures do. I think I would need something behaving similar to Uniform, but with the LLVM type consisting of some arbitrary combination of double and i64 types. Does implementing this seem reasonable?

This is the relevant extract from the ABI document (the first 2 paragraphs are the important ones):

Structs, unions, or other composite types are treated as a sequence of doublewords,
and are passed in integer or floating point registers as though they were simple
scalar parameters to the extent that they fit, with any excess on the stack packed
according to the normal memory layout of the object. More specifically:

  • Regardless of the struct field structure, it is treated as a sequence of 64-bit
    chunks. If a chunk consists solely of a double float field (but not a double,
    which is part of a union), it is passed in a floating point register. Any other
    chunk is passed in an integer register.
  • A union, either as the parameter itself or as a struct parameter field, is treated
    as a sequence of integer doublewords for purposes of assignment to integer
    parameter registers. No attempt is made to identify floating point components
    for passing in floating point registers.
  • Array fields of structs are passed like unions. Array parameters are passed by
    reference (unless the relevant language standard requires otherwise).
  • Right-justifying small scalar parameters in their save area slots
    notwithstanding, struct parameters are always left-justified. This applies both
    to the case of a struct smaller than 64 bits, and to the final chunk of a struct
    which is not an integral multiple of 64 bits in size. The implication of this rule is
    that the address of the first chunk’s save area slot is the address of the struct,
    and the struct is laid out in the save area memory exactly as if it were allocated
    normally (once any part in registers has been stored to the save area). [These
    rules are analogous to the o32-bit ABI treatment – only the chunk size and the
    ability to pass double fields in floating point registers are different.]
@kennytm kennytm added A-ffi Area: Foreign Function Interface (FFI) O-MIPS Target: MIPS processors C-bug Category: This is a bug. labels Jan 9, 2018
kennytm added a commit to kennytm/rust that referenced this issue Feb 25, 2018
rustc_trans: rewrite mips64 ABI code

This PR rewrites the ABI handling code for 64-bit MIPS and should fix various FFI issues including rust-lang#47290.

To accomodate the 64-bit ABI I have had to add a new `CastTarget` variant which I've called `Chunked` (though maybe this isn't the best name). This allows an ABI to cast to some arbitrary structure of `Reg` types. This is required on MIPS which might need to cast to a structure containing a mixture of `i64` and `f64` types.
@jcowgill
Copy link
Contributor Author

jcowgill commented Mar 6, 2018

This was fixed by #47964, but I forgot to reference this issue in the PR.

@jcowgill jcowgill closed this as completed Mar 6, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-ffi Area: Foreign Function Interface (FFI) C-bug Category: This is a bug. O-MIPS Target: MIPS processors
Projects
None yet
Development

No branches or pull requests

2 participants