rustc_trans: clobber $1 (aka $at) on mips

[jcowgill]

This copies what clang does. There is a long explanation as to why this is needed in the clang source (tools/clang/lib/Basic/Targets/Mips.h).

[rust-highfive]

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

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

[eddyb]

r? @nagisa or @rkruppe

[varkor]

How straightforward would it be to address the FIXME in the Clang comment, for Rust: i.e. only clobbering $1 for inline assembly that makes use of it?

[jcowgill]

Write your own assembly parser? If you can determine $1 is never used directly and the assembly contains no macros (you could hardcode a list of "known good" instructions to be safe), you can issue a .set at and .set nomacro to override LLVM's defaults and then remove $1 from the clobber list.

[jcowgill]

I guess this also depends on how much Clang / GCC compatability Rust should have. You could require the user to mark any inline assembly blocks which use macros or clobber $1, and any blocks without those markers would get the .set at and .set nomacro directives added and could avoid the $1 clobber.

[hanna-kruppe]

@bors r+

I think this PR is fine as-is and will just merge it. Nice catch and thanks for the PR @jcowgill!

How straightforward would it be to address the FIXME in the Clang comment, for Rust: i.e. only clobbering $1 for inline assembly that makes use of it?

No more straightforward than addressing it in Clang would be. Which is to say, apparently not straightforward enough to just do it.

There's also the question of how beneficial this would be. I don't think it's very useful. It doesn't seem like it would affect codegen quality (on MIPS you have plenty of registers, so at worst you lose one of many registers around inline asm) – and even if it does, at worst we're as bad as Clang and GCC, which is good company to be in.

Pushing the obligation to mark it as clobbered on the user also seems bad. A subtle difference from GCC might cause silent bugs when porting code, and this particular requirement seems rather onerous (since you have to mark as clobbered any time you use a pseudo instruction).

[bors]

📌 Commit d8e4142 has been approved by rkruppe

[nagisa]

Please add/copy/link the comment from Clang to the source code.

I can see the clobber being more important for Clang, as it strives to be compatible with code written for GCC and it is quite likely that there were a project or two that (ab)used $1 incorrectly.

This is not really the problem in Rust as it neither exposes gcc-like inline assembly, nor it aims to compile legacy C projects :) That being said, I see no reason against landing it either.

[hanna-kruppe]

@nagisa

Please add/copy/link the comment from Clang to the source code.

I considered requesting this but we already have a comment saying we're basically copying Clang here. It would be nice to have the rationale reproduced inline, but I don't think it's that important. If you want to block on this then feel free to r-.

[bors]

💡 This pull request was already approved, no need to approve it again.

• There's another pull request that is currently being tested, blocking this pull request: Replace all const evaluation with miri #46882

[bors]

📌 Commit d8e4142 has been approved by rkruppe

[bors]

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