Explicitly pass ABI flags for MIPS.

[jhb]

Summary

Explicitly pass ABI flags for MIPS.

Similar to r308711, set the desired ABI explicitly for MIPS targets.
This better supports external toolchains whose default ABI may differ
from the desired ABI. In particular, one can now build o32, n32, and
n64 binaries with mips-gcc from ports after this change.

Similarly, explicitly pass the desired linker emulation already defined
in LD_EMULATION when linking kernels and kernel modules. This permits
linking kernels with a non-default ABI.

Test Plan

• tested mips o32/n64 kernel and worlds with gcc4.2 (in-tree) and gcc 6.2 (from ports).
• still waiting on a full universe build (in progress) to ensure the LD_EMULATION doesn't result in any regressions
• Note that gcc6.2 on mips requires an __ffssi2() builtin that is not included in our current C runtime. I have a separate patch to export that from libc.

[emaste]

Overall LGTM, two minor comments inline.

share/mk/bsd.cpu.mk

317–332

Might be worth factoring out the -EL/-EB/-mabi into MIPS_FLAGS or something rather than repeating groups of ACFLAGS/AFLAGS/CFLAGS/LDFLAGS n times?

share/mk/sys.mk

224–225

update comment?

[imp]

I generally like this change. We tried very hard to require the system compiler to do the right thing for two reasons back in the day. First, we wanted to make sure a naked cc could just build things so ports would work better. Second, there were several tricky, hard-coded at the time, bits in different builds that got the wrong ABI tags and so wouldn't link (hack.so?) that were a pita to track down... If the latter has been solved, the former does remain. However, I think it's better that we're explicit since it enables a wider range of compiler to build the base system, and the problem of using those compilers with ports is orthogonal. We shouldn't require it any longer since it is standing in the way of making things better. I regret that we decided to do what we did back in the day since it has held back the external-toolchain work on mips a bit.

share/mk/bsd.cpu.mk

317–332

At one point I used _CPUFLAGS for this. Bit I had also eliminated NO_CPU_CFLAGS as well...

Do all these flags need to be set? ACFLAGS is set from cflags, so wouldn't that be duplicative, for example?

We used to require that the ABI of the compiler match the target so things like ports would just work on the target system. There's a risk that the system compiler won't produce the right binaries now, but I think time has shown that's a less severe problem than issues we have because we don't specify everything.

sys/conf/Makefile.arm

63 ↗

(On Diff #26521)

This may be a good change, but it's unrelated, as do all the LD_EMULATION changes... While they are similar to the MIPS stuff, perhaps they should be submitted separately.

But on the other hand, maybe it's time to take a look at the duplication between the Makefiles and see if there might be a way to do it better?

[jhb]

share/mk/bsd.cpu.mk

317–332

Hmm, ACFLAGS does not appear to be set from CFLAGS. In sys.mk:

AS              ?=      as
AFLAGS          ?=
ACFLAGS         ?=

I had originally only set these for buildworld/buildkernel in Makefile.inc1 btw, but kan@'s changes in rS308711 are what led me to move them here to be beside the -EL/-EB.

I'm fine doing a _MIPS_FLAGS if we agree that is cleaner.

376

This is required for external GCC to compile assembly files that use ll/sc since external GCC requires an explicit CPUTYPE for o32 (it's default -march= for o32 is mips2)

sys/conf/Makefile.arm

63 ↗

(On Diff #26521)

My intention is to commit LD_EMULATION as a separate commit, but I think for review it provides better context to review the two commits together. This is the same idea as the explicit -mabi for the compiler driver. In the case of devel/mips-binutils, mips-freebsd-ld assumes o32 and requires explicit -m for n32 or n64. devel/mips64-binutils defaults to n64, but we don't have a 'mipsn32-binutils' or 'mipsn32-gcc'. Also, with these changes, you can just use mips-gcc and mips-binutils for all 3 ABIs now (so arguably we could remove the mips64-* toolchain packages).

As far as the duplication of Makefile.arm vs kern.pre.mk, that doesn't seem easy to untangle. OTOH, I noticed that Makefile.arm is missing some of the recent changes to SYSTEM_LD from kern.pre.mk (which I might also commit separate from LD_EMULATION).

[imp]

share/mk/bsd.cpu.mk

317–332

Look at bsd.suffixes.mk (and others):

.S.o:

${CC:N${CCACHE_BIN}} ${CFLAGS} ${ACFLAGS} -c ${.IMPSRC} -o ${.TARGET}
${CTFCONVERT_CMD}

So ACFLAGS is additional flags for assembler not contained in CFLAGS.... kan's changes were wrong as well...

[jhb]

share/mk/bsd.cpu.mk

317–332

So this breaks lib/csu/mips/Makefile which uses:

crt1.s: crt1.c
        ${CC} ${CFLAGS} -S -o ${.TARGET} ${.CURDIR}/crt1.c
        sed ${SED_FIX_NOTE} ${.TARGET}

crt1.o: crt1.s
        ${CC} ${ACFLAGS} -c -o ${.TARGET} crt1.s

And apparently that _can't_ use CFLAGS directly but has to only use ACFLAGS per rS234502.

376

Actually, this is the same issue as above for lib/csu/<foo>/Makefile.

[jhb]

Adding dim@ in case he has better ideas on the ACFLAGS mess due to lib/csu Makefiles. It seems if we had a real llvm-as then we could just use ${AS} here which would be the cleaner solution, but presumably we won't have that anytime soon.

[jhb]

• Fix linker emulation for armeb.
• Include CFLAGS with ACFLAGS when compiling crt assembly files via CC.
• Drop ACFLAGS that duplicate CFLAGS from FreeBSD/mips.

[jhb]

• Expand comments on LDFLAGS vs _LDFLAGS.

[jhb]

lib/csu/mips/Makefile

30

This worked on mips builds with GCC 4.2.1 and 6.3. If we think this approach is what we'd like to do I can expand it to other architectures.

share/mk/bsd.cpu.mk

368

This is likely due to the same csu Makefile issues and should probably be removed eventually as well.

sys/conf/kern.mk

261 ↗

(On Diff #26999)

Tripped over this during universe. No armeb kernels currently use firmware blobs which is why this doesn't break currently.

[imp]

OK. I'm cool with this... As we've noted there's issues here, but other things need to be fixed before we can address the criticisms. And that isn't important enough to hold up this patch for.

lib/csu/mips/Makefile

30

I'm game for this in the short term, though it is still wrong.

However, I have a notion for how to fix this more generically to remove the need to SED, which would remove this as a problem...

share/mk/bsd.cpu.mk

317–332

Yea, that usage is wrong, but sadly necessary.

[jhb]

• Rebase to reflect currently committed bits.
• Add CFLAGS (without -g) alongside ACFLAGS in the other csu Makefiles.

[emaste]

Probably don't want to commit multiple pieces with the same Differential Revision since it confuses Phabricator.

[jhb]

It actually lists all the commits up top at the start of the review, so it seems to not be that confused. In terms of posterity looking at this in the future, they will see all of the commits and the commentary on the intermediate versions, etc.

[imp]

Looks good to me. Some minor tweaks will be needed for some other issues, but those are beyond the scope of these changes so this is good to go.