Page MenuHomeFreeBSD

Fix MIPS CSU compilation w/ LLVM
ClosedPublic

Authored by kevans on Sep 18 2019, 2:15 AM.

Details

Summary

GCC issues the warning, but with LLVM it is fatal- no matching .cprestore with .cpload. We do similar shenanigans in machine/profile.h:_mcount -- a dummy .cprestore. This seems to work in practice.

PR: 233361

Diff Detail

Repository
rS FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

kevans created this revision.Sep 18 2019, 2:15 AM
imp added a comment.Sep 25 2019, 8:14 PM

How do you know offset 12 on the stack is a good place to store this? I'd expect there to be something like:
cprestore will store gp to <offset>(sp) and then load it from there after every jal operation to restore gp.

I'm also at a loss for how this code could possibly work: there's no delay slot instruction after the JAL and there's no jr ra. But I'm willing to suspend my disbelief on this one: All hail mips assembler magic.

At the very least we need to add a addiu sp,sp,-16 after the .cpload and before the .cprestore. But then we'd need a addiu sp,sp,16 after the above missing jr ra....

imp added a comment.Sep 25 2019, 8:16 PM

At least according to https://techpubs.jurassic.nl/manuals/0530/developer/Cplr_PTG/sgi_html/apa.html which appears to be the reproduction of the sgi assembler manual from whence all this stuff came.

If prefer to just spell out the assembly rather than just the magic cp* macros.
Apparently they place a $gp reload after every call: Additionally, it causes the assembler to emit:

lw     gp,offset(sp)

after every jump-and-link (jal) operation (but not after a branch-and-link (bal) operation), thereby restoring the gp register after function calls. The programmer is responsible for allocating the stack space for the gp.

And it does seem like there is a nop missing after the JAL since we do a .set noreorder.

Do we actually use this code for MIPS? I think cheribsd still uses the old CSU bits.

imp added a comment.Sep 26 2019, 4:34 PM

If prefer to just spell out the assembly rather than just the magic cp* macros.

Except that's not really an option. These are extra special directives, in addition to being a macro and there has to be matched pairs.

Apparently they place a $gp reload after every call: Additionally, it causes the assembler to emit:

lw     gp,offset(sp)

after every jump-and-link (jal) operation (but not after a branch-and-link (bal) operation), thereby restoring the gp register after function calls. The programmer is responsible for allocating the stack space for the gp.

Correct, which we've not done here, it seems from my reading of their use, but with so many levels of indirections, it's hard to know for sure.

And it does seem like there is a nop missing after the JAL since we do a .set noreorder.

Do we actually use this code for MIPS? I think cheribsd still uses the old CSU bits.

I think that we do. Otherwise we wouldn't have hit this error we need to fix...

kevans updated this revision to Diff 62980.Oct 7 2019, 2:30 AM

There seems to be some magic at play here, and I don't understand it. I've added the stack space in a reasonable spot, this still builds and passes the csu kyua tests just happily enough for me.

arichardson accepted this revision.Jan 2 2020, 2:49 PM

Looks good. Compiles for me with latest LLVM.

lib/csu/mips/crt.h
54 ↗(On Diff #62980)

I guess there is also a nop missing here.

This revision is now accepted and ready to land.Jan 2 2020, 2:49 PM
kevans added inline comments.Jan 2 2020, 2:53 PM
lib/csu/mips/crt.h
54 ↗(On Diff #62980)

I'll add that pre-commit while I'm in the area.

This revision was automatically updated to reflect the committed changes.