[MIPS64] [MIPS] Modifications in support of clang enabled builds + IAS
Needs ReviewPublic

Authored by sbruno on Feb 1 2017, 4:25 PM.

Details

Summary

Minimal changes stolen from BERI and from John's mips work to build
and support clang-head builds of FreeBSD MIPS64 targets

Requires clang built from head with the following reviews applied:
https://reviews.llvm.org/D16807
https://reviews.llvm.org/D29032
https://reviews.llvm.org/D29218

Diff Detail

Repository
rS FreeBSD src repository
Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 7230
Build 7400: arc lint + arc unit
sbruno updated this revision to Diff 24628.Feb 1 2017, 4:25 PM
sbruno retitled this revision from to [MIPS64] Modifications in support of clang enabled builds + IAS.
sbruno updated this object.
sbruno edited the test plan for this revision. (Show Details)
sbruno added reviewers: emaste, brooks, rwatson, jhb.
sbruno updated this revision to Diff 24634.Feb 1 2017, 5:53 PM

Add a few more trivial updates for clang enabled builds.

sbruno added inline comments.Feb 1 2017, 6:02 PM
contrib/binutils/bfd/elf64-mips.c
1694

All of these changes are stolen from CHERI. Thanks guys!

2253

Stolen from CHERI

2286

Stolen from CHERI

2312

Stolen from CHERI

contrib/gcc/config/mips/mips.h
2724

suggested by upstream clang maintainers to deal with our old and busted binutils failing to link. This appears functional

lib/csu/mips/crti.S
6

Rip off John's tree here.

libexec/rtld-elf/mips/rtld_start.S
137

This syntax doesn't work with clang. The "fixed" version being proposed (thanks Ed!) compiles with both gcc 4.2 and clang.

sys/boot/mips/beri/boot2/Makefile
66

Clang rejects this -N here. I'm unsure what its for. Maybe brooks has an idea here?

sys/boot/mips/beri/boot2/relocate.S
35 ↗(On Diff #24634)

clang defaults to nobopt. The lack of parsing of .set nobopt will be fixed upstream into a no-opt, but for now, just wrap it out.

usr.bin/xlint/xlint/xlint.c
412 ↗(On Diff #24634)

-Wtraditional doesn't appear to be a thing with clang. Maybe gcc supports -Traditional and all of this needs to be just changed? But, for now, this works.

jhb edited edge metadata.Feb 1 2017, 8:38 PM

One question is how are you handling ABICALLS? Does upstream clang now define this when -mabicalls is present but not when -mno-abicalls is specified? I have a hack in my branch to key off -fpic to make this work.

In D9407#194412, @jhb wrote:

One question is how are you handling ABICALLS? Does upstream clang now define this when -mabicalls is present but not when -mno-abicalls is specified? I have a hack in my branch to key off -fpic to make this work.

Upstream clang review in progress here:

https://reviews.llvm.org/D29032

sbruno updated this revision to Diff 24701.Feb 3 2017, 9:05 PM
sbruno edited edge metadata.

Completely disable lib32 in the mips64+clang case. None of the variables I have access
to allow me to do this from the environment.

Add some assembly work arounds for clang built kernels.

sbruno added a comment.Feb 3 2017, 9:10 PM

https://reviews.llvm.org/D29218 is pending. This should be a prequisite before we move forward with base switching over to clang-mips64.

This patch will need to be modified a bit more, but you get the idea here to enable IAS for mips64

Index: lib/Driver/ToolChains.cpp
===================================================================
--- lib/Driver/ToolChains.cpp   (revision 294008)
+++ lib/Driver/ToolChains.cpp   (working copy)
@@ -2920,8 +2920,8 @@
   case llvm::Triple::systemz:
   case llvm::Triple::mips:
   case llvm::Triple::mipsel:
+  case llvm::Triple::mips64:
     return true;
-  case llvm::Triple::mips64:
   case llvm::Triple::mips64el:
     // Enabled for Debian mips64/mips64el only. Other targets are unable to
     // distinguish N32 from N64.
@@ -3667,6 +3667,7 @@
   // When targeting 32-bit platforms, look for '/usr/lib32/crt1.o' and fall
   // back to '/usr/lib' if it doesn't exist.
   if ((Triple.getArch() == llvm::Triple::x86 ||
+       Triple.getArch() == llvm::Triple::mips ||
        Triple.getArch() == llvm::Triple::ppc) &&
       D.getVFS().exists(getDriver().SysRoot + "/usr/lib32/crt1.o"))
     getFilePaths().push_back(getDriver().SysRoot + "/usr/lib32");

h0h0

https://gist.github.com/seanbruno/dc5c1e2145199d8101f1a665f1d8d2df

This lets mips32 build and boot now. :-)

Still no idea what's up with getting the handoff to userland correctly.

sbruno retitled this revision from [MIPS64] Modifications in support of clang enabled builds + IAS to [MIPS64] [MIPS] Modifications in support of clang enabled builds + IAS.Feb 3 2017, 11:34 PM
jhb added inline comments.Feb 3 2017, 11:51 PM
Makefile.inc1
607

Why not just trim the default -march= setting in Makefile.libcompat instead based on COMPILER_TYPE?

sbruno added inline comments.Feb 4 2017, 12:15 AM
Makefile.inc1
607

My hope is that clang will "grow" -march=mips3 support and I didn't want to break the existing gcc 4.2.1 build at the moment.

sbruno updated this revision to Diff 24705.Feb 4 2017, 12:27 AM

Clang has "grown" support for .set nobopt in that it always does this and just silently
ignores the directive. :-)

Remove the modified xlint.c diff as I'm just disabling the build alltogether.

sbruno added a comment.Feb 4 2017, 7:04 PM

make.conf I'm using:

MALLOC_PRODUCTION=y
NO_WERROR=y
TARGET=mips
TARGET_ARCH=mips
KERNCONF=MALTA64
XCC=/home/sbruno/clang/build/bin/clang
XCPP=/home/sbruno/clang/build/bin/clang-cpp
XCXX=/home/sbruno/clang/build/bin/clang++
XCFLAGS=-Wno-typedef-redefinition -Wno-pointer-sign -Wno-expansion-to-defined -Wno-address-of-packed-member

src.conf I'm using:

WITHOUT_CLANG="YES"
WITHOUT_CLANG_EXTRAS="YES"
WITHOUT_CLANG_FULL="YES"
WITHOUT_CLANG_BOOTSTRAP="YES"
WITHOUT_GCC="YES"
WITHOUT_GDB="YES"
WITHOUT_LIB32="YES"
WITHOUT_COMPAT32="YES"
WITHOUT_TESTS="YES"
WITHOUT_RESCUE="YES"
WITHOUT_LIB32="YES"
brooks edited edge metadata.Feb 5 2017, 4:13 PM

Most things are looking good. A few comments.

libexec/rtld-elf/mips/rtld_start.S
141

In this version there's no load from a0 any more so this is not at all the same thing. You need to restore

ld        a0, 8(a0)                /* object = pltgot[1] */
sys/boot/mips/beri/boot2/Makefile
66

I think I'd convert this to -Wl,-N. That will cause it to appear in _LDFLAGS as -N and would be passed through clang.

sys/dev/gxemul/disk/gxemul_disk.c
174 ↗(On Diff #24705)

This should probably be a separate commit since it's a flat out bug.

sbruno added inline comments.Feb 6 2017, 6:37 AM
sys/boot/mips/beri/boot2/Makefile
66

No luck, but I may not understand what I should be doing here.

/home/sbruno/clang/build/bin/clang -Wno-typedef-redefinition -Wno-pointer-sign -Wno-expansion-to-defined -Wno-address-of-packed-member -target mips64-unknown-freebsd12.0 --sysroot=/var/tmp/sbruno/mips.mips64/home/sbruno/fbsd_head/tmp -B/v
ar/tmp/sbruno/mips.mips64/home/sbruno/fbsd_head/tmp/usr/bin -nostdlib -static -N -G0 -L/home/sbruno/fbsd_head/sys/boot/mips/beri/boot2 -EB -nostdlib -T /home/sbruno/fbsd_head/sys/boot/mips/beri/boot2/jtagboot.ldscript -o jtagboot      sta
rt.o boot2.o altera_jtag_uart.o cfi.o sdcard.o /var/tmp/sbruno/mips.mips64/home/sbruno/fbsd_head/sys/boot/mips/beri/boot2/../../../../../lib/libstand/libstand.a
clang-4.0: error: unknown argument: '-N'
Index: sys/boot/mips/beri/boot2/Makefile
===================================================================
--- sys/boot/mips/beri/boot2/Makefile   (revision 313320)
+++ sys/boot/mips/beri/boot2/Makefile   (working copy)
@@ -63,7 +63,7 @@

 LDFLAGS=       -nostdlib                       \
                -static                         \
-               -N                              \
+               -Wl,-N                          \
                -G0                             \
                -L${.CURDIR}
sbruno marked an inline comment as done.Feb 6 2017, 11:38 AM
sbruno added inline comments.
sys/dev/gxemul/disk/gxemul_disk.c
174 ↗(On Diff #24705)

Committed at svn 313338

sbruno updated this revision to Diff 24796.Feb 6 2017, 11:40 AM
sbruno marked an inline comment as done.
sbruno edited edge metadata.

Drop gxemul change already committed to head.

Restore missing ld instruction as per review comments.

sbruno updated this object.Feb 6 2017, 2:15 PM
br added a subscriber: br.Feb 6 2017, 2:19 PM
sbruno added a comment.Feb 6 2017, 2:44 PM

And you'll need this patch to your clang tree as well, still unsure how to upstream this.

Index: lib/Driver/ToolChains.cpp
===================================================================
--- lib/Driver/ToolChains.cpp   (revision 294182)
+++ lib/Driver/ToolChains.cpp   (working copy)
@@ -2920,9 +2920,9 @@
   case llvm::Triple::systemz:
   case llvm::Triple::mips:
   case llvm::Triple::mipsel:
-    return true;
   case llvm::Triple::mips64:
   case llvm::Triple::mips64el:
+    return true;
     // Enabled for Debian mips64/mips64el only. Other targets are unable to
     // distinguish N32 from N64.
     if (getTriple().getEnvironment() == llvm::Triple::GNUABI64)
@@ -3667,6 +3667,7 @@
   // When targeting 32-bit platforms, look for '/usr/lib32/crt1.o' and fall
   // back to '/usr/lib' if it doesn't exist.
   if ((Triple.getArch() == llvm::Triple::x86 ||
+       Triple.getArch() == llvm::Triple::mips ||
        Triple.getArch() == llvm::Triple::ppc) &&
       D.getVFS().exists(getDriver().SysRoot + "/usr/lib32/crt1.o"))
     getFilePaths().push_back(getDriver().SysRoot + "/usr/lib32");
sbruno marked an inline comment as done.Feb 6 2017, 3:11 PM
sbruno added inline comments.
libexec/rtld-elf/mips/rtld_start.S
141

Restored ld instruction.

simon.dardis_imgtec.com added inline comments.
libexec/rtld-elf/mips/rtld_start.S
137

That immediate is bigger than we normally take. It should be simple enough to extend IAS to accept it for N32 / N64.

sys/mips/mips/exception.S
193–195

I'll take another look at this.

emaste added inline comments.Feb 21 2017, 2:54 PM
share/mk/src.opts.mk
224–225

FYI in the clang400-import branch I've turned on Clang by default everywhere when the host compiler is C++11 ( rS312776), although it's still not /usr/bin/cc on MIPS.

jhb added inline comments.Apr 11 2017, 10:35 PM
contrib/gcc/config/mips/mips.h
2724

I suspect this is actually a bug in clang. The non-local function (frame_dummy) in crtstuff.c is declared 'static' so the .local should in theory be redundant. For reference, without this patch, these are the relocations for crtbeginS.o compiled with GCC 6.3 for O32:

RELOCATION RECORDS FOR [.init]:
OFFSET   TYPE              VALUE 
00000008 R_MIPS_HI16       _gp_disp
0000000c R_MIPS_LO16       _gp_disp
00000014 R_MIPS_GOT16      .text
00000018 R_MIPS_LO16       .text
0000001c R_MIPS_JALR       frame_dummy

and these are the relocations for recent-ish clang (post 4.0):

RELOCATION RECORDS FOR [.init]:
OFFSET   TYPE              VALUE 
00000008 R_MIPS_HI16       _gp_disp
0000000c R_MIPS_LO16       _gp_disp
00000014 R_MIPS_CALL16     frame_dummy

(frame_dummy is marked static in crtstuff.c) Also note that I get this identical error with a much newer binutils (2.27) so it's not a matter of our old binutils, but clang emitting the wrong thing.

emaste added inline comments.Jun 28 2017, 1:18 PM
lib/csu/mips/crti.S
6

Do you know if there's any upstream discussion of this issue?

jhb added inline comments.Jun 28 2017, 8:14 PM
lib/csu/mips/crti.S
6

I've not tried any or opened a bug, etc. It's also not clear that this is really correct for the *hf targets either as this is specifying a soft-float ABI (IIUC).

emaste added inline comments.Dec 19 2017, 2:54 PM
usr.bin/Makefile
287–292

This part is now gone in HEAD

How much of this do we still need?

jhb added a comment.Feb 5 2018, 5:57 PM

I have a few remaining pieces of this in my branch at https://github.com/bsdjhb/freebsd/compare/master...bsdjhb:mips_xbuild Note that I'm not trying to use the in-tree binutils though and am requiring ld.bfd so I don't use the binutils patches.