Make rlib metadata strip works with MIPSr6 architecture

[Fearyncess]

Because MIPSr6 has many differences with previous MIPSr2 arch, the previous rlib metadata stripping code in rustc_codegen_ssa is only for MIPSr2/r3/r5 (which share the same elf e_flags).

This commit fixed this problem. It makes rustc_codegen_ssa happy when compiling rustc for MIPSr6 target or hosts.

e_flags REF: https://github.com/llvm/llvm-project/blob/e356027016c6365b3d8924f54c33e2c63d931492/llvm/include/llvm/BinaryFormat/ELF.h#L562

[rust-highfive]

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @cjgillot (or someone else) soon.

Please see the contribution instructions for more information.

[cjgillot]

Thanks for the PR @Fearyncess.
Unfortunately, I don't have much knowledge of this area of the compiler.
r? @alexcrichton

[alexcrichton]

It looks like the only addition here is EF_MIPS_NAN2008, so could the code be refactored to not have duplicate flags on each branch and instead only add that flag as necessary? Otherwise seems fine to me, but if you're up for it it'd probably be good to update the object crate dependency from other crates to ideally only use one version instead of introducing another.

[Fearyncess]

It looks like the only addition here is EF_MIPS_NAN2008, so could the code be refactored to not have duplicate flags on each branch and instead only add that flag as necessary? Otherwise seems fine to me, but if you're up for it it'd probably be good to update the object crate dependency from other crates to ideally only use one version instead of introducing another.

EF_MIPS_ARCH_{64,32}R6 detection is needed. If not, linker will treat the extracted rlib meta as MIPS r2 binaries (however the rlibs compiled using MIPS r6 target). They are completely different. It will be failed when linking then.

[alexcrichton]

Er yes the other flags are needed as well, but it looks like these targets also need the EF_MIPS_NAN2008 over the existing flags that were already configured. If that's the case I think the patch here can be a bit smaller with less duplicating by only adding EF_MIPS_NAN2008 for these specific targets instead of re-listing all the existing flags twice.

[Fearyncess]

Er yes the other flags are needed as well, but it looks like these targets also need the EF_MIPS_NAN2008 over the existing flags that were already configured. If that's the case I think the patch here can be a bit smaller with less duplicating by only adding EF_MIPS_NAN2008 for these specific targets instead of re-listing all the existing flags twice.

I've checked that MIPSr5 (which has MSA enforced) and Loongson 4000 series is used NaN2008 encoding by default, but does it will be a ABI break problem? https://web.archive.org/web/20180830093617/https://dmz-portal.mips.com/wiki/MIPS_ABI_-_NaN_Interlinking

[alexcrichton]

Sorry I don't actually know much about mips to the point that I can comment on the technical bits of this patch, I'm just commenting on the code structure itself and how it can be refactored to have a little less duplication than it currently has. Otherwise I would trust you that the flags are working as intended and are necessary for the target that you're working with.

[chenx97]

9859b81 is the initial support for version 6 of MIPS. @wzssyqa Could you help review this pull request?

It looks like the only addition here is EF_MIPS_NAN2008, so could the code be refactored to not have duplicate flags on each branch and instead only add that flag as necessary? Otherwise seems fine to me, but if you're up for it it'd probably be good to update the object crate dependency from other crates to ideally only use one version instead of introducing another.

@alexcrichton I would love to help factor out the duplication. Should I make e_flags be common flags | the if-else block of r6 detection? e.g.

let e_flags = elf::EF_MIPS_CPIC
    | elf::EF_MIPS_PIC
    | if sess.target.options.cpu.contains("r6") {
        // copied from the `e_flags` field of the object
        // generated by `mipsisa64r6el-linux-gnuabi64-gcc foo.c -c`
        elf::EF_MIPS_ARCH_64R6 | elf::EF_MIPS_NAN2008
    } else {
        elf::EF_MIPS_ARCH_64R2
    };

Also, regarding having NaN2008 on MIPSr5, I think this should be out of the scope of this pull request. This pull request is meant to only deal with mipsisa{32,64}r6el-related targets. What we need to make sure here is that this pull request won't change anything for those Tier-2 MIPS targets, targeting chips from MIPSr2 to MIPSr5.

[chenx97]

@alexcrichton Sorry for suddenly jumping into the conversation. I'm a colleague of the author of this pull request, so I asked the questions on behalf of our team. We are at CIP United and we will continue to improve Rust's support for the MIPSr6 family, and hopefully make it tier-2 in the future (which also means making real MIPSr6 hardware, and we look forward to it).😊

[alexcrichton]

My review of this PR is basically purely stylistic, not really technical. The MIPS flags are all duplicated where the presence of the r6 feature implies EF_MIPS_NAN2008 is or-ed into the list of flags. I would stylistically request less duplication, along the lines of what #90001 (comment) mentions.

[bors]

☔ The latest upstream changes (presumably #91604) made this pull request unmergeable. Please resolve the merge conflicts.

[bors]

☔ The latest upstream changes (presumably #89819) made this pull request unmergeable. Please resolve the merge conflicts.

[Fearyncess]

My review of this PR is basically purely stylistic, not really technical. The MIPS flags are all duplicated where the presence of the r6 feature implies EF_MIPS_NAN2008 is or-ed into the list of flags. I would stylistically request less duplication, along the lines of what #90001 (comment) mentions.

@alexcrichton

i have make the code more simpilified and update the dep of rustc_codegen_ssa for it. it seems works well

[alexcrichton]

@bors: r+

[bors]

📌 Commit cf36c21 has been approved by alexcrichton