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

32-bit mips target emits 64-bit instructions #104985

Open
he32 opened this issue Nov 27, 2022 · 10 comments
Open

32-bit mips target emits 64-bit instructions #104985

he32 opened this issue Nov 27, 2022 · 10 comments
Labels
A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. O-MIPS Target: MIPS processors T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@he32
Copy link
Contributor

he32 commented Nov 27, 2022

I am initially a little hesitant to characterize this as a bug, or whether it is just me asking for help...

I recently added support for the mipsel-unknown-netbsd target to our rust package in our pkgsrc-wip github repository, and based on this I managed to cross-build a bootstrap kit for that target. However, the resulting binaries fail to run on NetBSD's 32-bit ports, which use o32 ABI and have in general turned off support for 64-bit instructions.

The target spec calls for emitting instructions adhering to "mips3", which I think excludes any 64-bit instructions; the rustc target spec looks like this:

use crate::abi::Endian;
use crate::spec::{Target, TargetOptions};

pub fn target() -> Target {
    let mut base = super::netbsd_base::opts();
    base.max_atomic_width = Some(32);
    base.cpu = "mips3".into();

    Target {
        llvm_target: "mipsel-unknown-netbsd".into(),
        pointer_width: 32,
        data_layout: "e-m:m-p:32:32-i8:8:32-i16:16:32-i64:64-n32-S64".into(),
        arch: "mips".into(),
        options: TargetOptions {
            mcount: "__mcount".into(),
            endian: Endian::Little,
            ..base
        },
    }
}

However, despite this, we end up with 64-bit-only instructions in the output, ref.

q: {138} objdump -ldr /usr/pkg/bin/cargo | grep daddiu 
   dcbbc:       64010001        daddiu  at,zero,1
   dceec:       64010001        daddiu  at,zero,1
   dcf88:       64010001        daddiu  at,zero,1
   dd410:       64020001        daddiu  v0,zero,1
   dd428:       64420001        daddiu  v0,v0,1
   dd8b0:       64010001        daddiu  at,zero,1
   dd948:       64010001        daddiu  at,zero,1
^C

and trying to run it results in an "Illegal Instruction" signal and a core dump.

Now, I'm also not certain exactly what to blame, whether it's LLVM itself, or the coupling between rustc and LLVM.

Anyway, hints to get this fixed would be much appreciated.

Oh, yes, this is with code based on rust 1.65.0.

@workingjubilee
Copy link
Contributor

Hmm, so I looked up the ISA version you mentioned with a quick search and

     The MIPS3 extensions to the MIPS instruction set, introduced in the R4000
     processors, are primarily to support 64-bit addresses and arithmetic and
     a larger floating point register set. The 64-bit addressing features are
     only supported on machines	running	a 64-bit kernel.  The 64-bit
     arithmetic	features are supported on all machines running IRIX 6.2	and
     later releases (R4K and later CPUs).

     The MIPS3 instruction set extensions provide the following	features:

     o	  64-bit integer registers, with a complete set	of instructions	to
	  perform 64-bit integer arithmetic operations.

     o	  64-bit addresses and pointers.  The R4000 family and later MIPS
	  processors support a 64-bit flat address space.

     o	  Thirty two 64-bit floating point registers.  The R4000 family
	  supports two floating	point register (FPR) modes, determining	how
	  many 64-bit FPRs are available:  16-FPR mode and 32-FPR mode.	 The
	  16-FPR mode is compatible with the R2000/R3000 and is	available
	  using	the default (-mips1) or	-mips2 compiler	options.  The 32-FPR
	  mode is not compatible with the MIPS1	or MIPS2 model.	 Hence it is
	  only supported by the	-mips3 and -mips4 options.

I am an expert on many other architectures and not so much on MIPS. Yet I feel uncertain if you are asking for what you actually want and want to make sure if that is indeed what you meant to ask for?

@workingjubilee workingjubilee added the O-MIPS Target: MIPS processors label Nov 28, 2022
@he32
Copy link
Contributor Author

he32 commented Nov 28, 2022

You may be right, and that it's not so much the CPU as the system which has made the choice to not enable the 64-bit instructions, even though my CPU supports them. Back to the drawing board.

@chenx97
Copy link
Contributor

chenx97 commented Feb 26, 2023

According to compiler/rustc_target, PSP's CPU is mips1, and PSX's CPU is mips2. Every other mips32-based target uses mips32r2 except for mipsisa32r6 and mipsisa32r6el. Currently LLVM-based toolchains appear to fail to support o32 for 64-bit CPUs (i.e. clang -march=mips3 -mabi=32), even though it, to some extent (i.e. following what gcc does), should support such combinations and emit no 64-bit-only instructions in this case, just like if you were generating code for x86 while specifying x86-64 as the march.

@workingjubilee workingjubilee added the A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. label Feb 26, 2023
@workingjubilee
Copy link
Contributor

Ah, so this is an insufficiency in LLVM's MIPS support? Then it can be reported upstream, but we're probably not going to make progress on fixing this here. Neither do we need to close it, however, as we can still track the issue until LLVM does fix it (or until it gets closed by something else, like e.g. implementing sufficient libgccjit support so people can get this by other means).

@chenx97
Copy link
Contributor

chenx97 commented Feb 26, 2023

My initial thought is just to use a truly 32-bit-only CPU for the target definition instead, but yeah this would be the ultimate fix.

@workingjubilee
Copy link
Contributor

workingjubilee commented Mar 3, 2023

Hmm, apparently a similar request was already vocalized but has been marked "wontfix", though its not clear if its an exact match or what the reason why was: llvm/llvm-project#18632

Eesh. I suppose you could ask them to reconsider.

@Nilstrieb Nilstrieb added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Apr 5, 2023
@chenx97
Copy link
Contributor

chenx97 commented May 6, 2023

Hmm, apparently a similar request was already vocalized but has been marked "wontfix", though its not clear if its an exact match or what the reason why was: llvm/llvm-project#18632

Eesh. I suppose you could ask them to reconsider.

It's fixed.

@workingjubilee
Copy link
Contributor

workingjubilee commented May 7, 2023

Not committed to HEAD yet. But when that changeset is accepted, we can accept a PR doing the requisite plumbing to allow defining a mips3_o32 target this way in its target definition. It may need to do a version check, though, for the yet-unreleased LLVM 17.

@Fearyncess
Copy link
Contributor

Not committed to HEAD yet. But when that changeset is accepted, we can accept a PR doing the requisite plumbing to allow defining a mips3_o32 target this way in its target definition. It may need to do a version check, though, for the yet-unreleased LLVM 17.

It has been merged, in a RC of LLVM 17.

@workingjubilee
Copy link
Contributor

Cool, that'll probably be in #114048 then.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. O-MIPS Target: MIPS processors T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

5 participants