arch-defines: also set LLD_EMUL if USE_LLD is set

classic Classic list List threaded Threaded
8 messages Options
Reply | Threaded
Open this post in threaded view
|

arch-defines: also set LLD_EMUL if USE_LLD is set

Stuart Henderson
Currently LLD_EMUL is only set for LLD_ARCHS (aarch64 amd64 arm i386).
A diff to unbreak mupdf build on mips64 requires linking with lld and this
variable is required. Rather than adding a custom LLD_EMUL definition in
mupdf let's just make the arch-defines.mk one reachable.

OK?

Index: arch-defines.mk
===================================================================
RCS file: /cvs/ports/infrastructure/mk/arch-defines.mk,v
retrieving revision 1.69
diff -u -p -1 -1 -r1.69 arch-defines.mk
--- arch-defines.mk 9 Nov 2019 15:08:09 -0000 1.69
+++ arch-defines.mk 4 Dec 2019 10:45:44 -0000
@@ -57,23 +57,23 @@ DEBUG_PACKAGES =
 DEBUG_FILES =
 .endif
 
 .if ${PROPERTIES:Mclang}
 LIBCXX = c++ c++abi pthread
 LIBECXX = c++ c++abi pthread
 .else
 LIBCXX = stdc++ pthread
 LIBECXX = estdc++>=17 pthread
 .endif
 
-.if ${PROPERTIES:Mlld}
+.if ${PROPERTIES:Mlld} || defined(USE_LLD) && ${USE_LLD:L} == yes
 # see llvm/tools/lld/ELF/Driver.cpp
 .  for A in aarch64.aarch64elf amd64.elf_amd64 arm.armelf i386.elf_i386 \
             mips64.elf64btsmip mips64el.elf64ltsmip powerpc.elf32ppc \
             sparc64.elf64_sparc
 .    if ${MACHINE_ARCH} == ${A:R}
 LLD_EMUL = -m${A:E}
 .    endif
 .  endfor
 .else
 LLD_EMUL =
 .endif

Reply | Threaded
Open this post in threaded view
|

Re: arch-defines: also set LLD_EMUL if USE_LLD is set

Marc Espie-2
On Wed, Dec 04, 2019 at 10:48:06AM +0000, Stuart Henderson wrote:

> Currently LLD_EMUL is only set for LLD_ARCHS (aarch64 amd64 arm i386).
> A diff to unbreak mupdf build on mips64 requires linking with lld and this
> variable is required. Rather than adding a custom LLD_EMUL definition in
> mupdf let's just make the arch-defines.mk one reachable.
>
> OK?
>
> Index: arch-defines.mk
> ===================================================================
> RCS file: /cvs/ports/infrastructure/mk/arch-defines.mk,v
> retrieving revision 1.69
> diff -u -p -1 -1 -r1.69 arch-defines.mk
> --- arch-defines.mk 9 Nov 2019 15:08:09 -0000 1.69
> +++ arch-defines.mk 4 Dec 2019 10:45:44 -0000
> @@ -57,23 +57,23 @@ DEBUG_PACKAGES =
>  DEBUG_FILES =
>  .endif
>  
>  .if ${PROPERTIES:Mclang}
>  LIBCXX = c++ c++abi pthread
>  LIBECXX = c++ c++abi pthread
>  .else
>  LIBCXX = stdc++ pthread
>  LIBECXX = estdc++>=17 pthread
>  .endif
>  
> -.if ${PROPERTIES:Mlld}
> +.if ${PROPERTIES:Mlld} || defined(USE_LLD) && ${USE_LLD:L} == yes
>  # see llvm/tools/lld/ELF/Driver.cpp
>  .  for A in aarch64.aarch64elf amd64.elf_amd64 arm.armelf i386.elf_i386 \
>              mips64.elf64btsmip mips64el.elf64ltsmip powerpc.elf32ppc \
>              sparc64.elf64_sparc
>  .    if ${MACHINE_ARCH} == ${A:R}
>  LLD_EMUL = -m${A:E}
>  .    endif
>  .  endfor
>  .else
>  LLD_EMUL =
>  .endif
>
Would there be a downside to just always set LLD_EMUL ?

By the way, I would probably code it differently to avoid the loop and
expensive computations:
_LLD_EMUL_aarch64 = aarch64elf
...
.if defined(_LLD_EMUL_${MACHINE_ARCH})
LLD_EMUL = -m${_LLD_EMUL_${MACHINE_ARCH}}
.else
LLD_EMUL =
.endif

Reply | Threaded
Open this post in threaded view
|

Re: arch-defines: also set LLD_EMUL if USE_LLD is set

Stuart Henderson
On 2019/12/04 12:17, Marc Espie wrote:

> On Wed, Dec 04, 2019 at 10:48:06AM +0000, Stuart Henderson wrote:
> > Currently LLD_EMUL is only set for LLD_ARCHS (aarch64 amd64 arm i386).
> > A diff to unbreak mupdf build on mips64 requires linking with lld and this
> > variable is required. Rather than adding a custom LLD_EMUL definition in
> > mupdf let's just make the arch-defines.mk one reachable.
> >
> > OK?
> >
> > Index: arch-defines.mk
> > ===================================================================
> > RCS file: /cvs/ports/infrastructure/mk/arch-defines.mk,v
> > retrieving revision 1.69
> > diff -u -p -1 -1 -r1.69 arch-defines.mk
> > --- arch-defines.mk 9 Nov 2019 15:08:09 -0000 1.69
> > +++ arch-defines.mk 4 Dec 2019 10:45:44 -0000
> > @@ -57,23 +57,23 @@ DEBUG_PACKAGES =
> >  DEBUG_FILES =
> >  .endif
> >  
> >  .if ${PROPERTIES:Mclang}
> >  LIBCXX = c++ c++abi pthread
> >  LIBECXX = c++ c++abi pthread
> >  .else
> >  LIBCXX = stdc++ pthread
> >  LIBECXX = estdc++>=17 pthread
> >  .endif
> >  
> > -.if ${PROPERTIES:Mlld}
> > +.if ${PROPERTIES:Mlld} || defined(USE_LLD) && ${USE_LLD:L} == yes
> >  # see llvm/tools/lld/ELF/Driver.cpp
> >  .  for A in aarch64.aarch64elf amd64.elf_amd64 arm.armelf i386.elf_i386 \
> >              mips64.elf64btsmip mips64el.elf64ltsmip powerpc.elf32ppc \
> >              sparc64.elf64_sparc
> >  .    if ${MACHINE_ARCH} == ${A:R}
> >  LLD_EMUL = -m${A:E}
> >  .    endif
> >  .  endfor
> >  .else
> >  LLD_EMUL =
> >  .endif
> >
> Would there be a downside to just always set LLD_EMUL ?

It needs to use different strings depending on the linker used:

arch ld.bfd ld.lld
i386 elf_i386_obsd elf_i386
amd64 elf_x86_64_obsd elf_amd64
aarch64 n/a aarch64elf
arm armelf_obsd armelf
mips64 elf64btsmip_obsd elf64btsmip
mips64el elf64ltsmip_obsd elf64ltsmip
powerpc elf32ppc_obsd elf32ppc
sparc64 elf64_sparc_obsd elf64_sparc64

Since ld.bfd has a default anyway, it seems easier to leave it unset
rather than carry a second table plus risk breakage (not that this is
really widely used; only mupdf, net/utox, www/mozplugger).

> By the way, I would probably code it differently to avoid the loop and
> expensive computations:
> _LLD_EMUL_aarch64 = aarch64elf
> ...
> .if defined(_LLD_EMUL_${MACHINE_ARCH})
> LLD_EMUL = -m${_LLD_EMUL_${MACHINE_ARCH}}
> .else
> LLD_EMUL =
> .endif
>

Oh that makes sense.

Index: arch-defines.mk
===================================================================
RCS file: /cvs/ports/infrastructure/mk/arch-defines.mk,v
retrieving revision 1.69
diff -u -p -u -1 -1 -r1.69 arch-defines.mk
--- arch-defines.mk 9 Nov 2019 15:08:09 -0000 1.69
+++ arch-defines.mk 4 Dec 2019 12:42:21 -0000
@@ -57,31 +57,36 @@ DEBUG_PACKAGES =
 DEBUG_FILES =
 .endif
 
 .if ${PROPERTIES:Mclang}
 LIBCXX = c++ c++abi pthread
 LIBECXX = c++ c++abi pthread
 .else
 LIBCXX = stdc++ pthread
 LIBECXX = estdc++>=17 pthread
 .endif
 
-.if ${PROPERTIES:Mlld}
+.if ${PROPERTIES:Mlld} || defined(USE_LLD) && ${USE_LLD:L} == yes
 # see llvm/tools/lld/ELF/Driver.cpp
-.  for A in aarch64.aarch64elf amd64.elf_amd64 arm.armelf i386.elf_i386 \
-            mips64.elf64btsmip mips64el.elf64ltsmip powerpc.elf32ppc \
-            sparc64.elf64_sparc
-.    if ${MACHINE_ARCH} == ${A:R}
-LLD_EMUL = -m${A:E}
-.    endif
-.  endfor
+_LLD_EMUL_aarch64 = aarch64elf
+_LLD_EMUL_amd64 = elf_amd64
+_LLD_EMUL_arm = armelf
+_LLD_EMUL_i386 = elf_i386
+_LLD_EMUL_mips64 = elf64btsmip
+_LLD_EMUL_mips64el = elf64ltsmip
+_LLD_EMUL_powerpc = elf32ppc
+_LLD_EMUL_sparc64 = elf64_sparc
+.endif
+
+.if defined(_LLD_EMUL_${MACHINE_ARCH})
+LLD_EMUL = -m${_LLD_EMUL_${MACHINE_ARCH}}
 .else
 LLD_EMUL =
 .endif
 
 # system version wide specifics
 _SYSTEM_VERSION = 1
 _SYSTEM_VERSION-aarch64 = 3
 _SYSTEM_VERSION-amd64 = 4
 _SYSTEM_VERSION-arm = 3
 _SYSTEM_VERSION-i386 = 2
 _SYSTEM_VERSION-mips64 = 1

Reply | Threaded
Open this post in threaded view
|

Re: arch-defines: also set LLD_EMUL if USE_LLD is set

Marc Espie-2
On Wed, Dec 04, 2019 at 12:45:46PM +0000, Stuart Henderson wrote:

> On 2019/12/04 12:17, Marc Espie wrote:
> > On Wed, Dec 04, 2019 at 10:48:06AM +0000, Stuart Henderson wrote:
> > > Currently LLD_EMUL is only set for LLD_ARCHS (aarch64 amd64 arm i386).
> > > A diff to unbreak mupdf build on mips64 requires linking with lld and this
> > > variable is required. Rather than adding a custom LLD_EMUL definition in
> > > mupdf let's just make the arch-defines.mk one reachable.
> > >
> > > OK?
> > >
> > > Index: arch-defines.mk
> > > ===================================================================
> > > RCS file: /cvs/ports/infrastructure/mk/arch-defines.mk,v
> > > retrieving revision 1.69
> > > diff -u -p -1 -1 -r1.69 arch-defines.mk
> > > --- arch-defines.mk 9 Nov 2019 15:08:09 -0000 1.69
> > > +++ arch-defines.mk 4 Dec 2019 10:45:44 -0000
> > > @@ -57,23 +57,23 @@ DEBUG_PACKAGES =
> > >  DEBUG_FILES =
> > >  .endif
> > >  
> > >  .if ${PROPERTIES:Mclang}
> > >  LIBCXX = c++ c++abi pthread
> > >  LIBECXX = c++ c++abi pthread
> > >  .else
> > >  LIBCXX = stdc++ pthread
> > >  LIBECXX = estdc++>=17 pthread
> > >  .endif
> > >  
> > > -.if ${PROPERTIES:Mlld}
> > > +.if ${PROPERTIES:Mlld} || defined(USE_LLD) && ${USE_LLD:L} == yes
> > >  # see llvm/tools/lld/ELF/Driver.cpp
> > >  .  for A in aarch64.aarch64elf amd64.elf_amd64 arm.armelf i386.elf_i386 \
> > >              mips64.elf64btsmip mips64el.elf64ltsmip powerpc.elf32ppc \
> > >              sparc64.elf64_sparc
> > >  .    if ${MACHINE_ARCH} == ${A:R}
> > >  LLD_EMUL = -m${A:E}
> > >  .    endif
> > >  .  endfor
> > >  .else
> > >  LLD_EMUL =
> > >  .endif
> > >
> > Would there be a downside to just always set LLD_EMUL ?
>
> It needs to use different strings depending on the linker used:
>
> arch ld.bfd ld.lld
> i386 elf_i386_obsd elf_i386
> amd64 elf_x86_64_obsd elf_amd64
> aarch64 n/a aarch64elf
> arm armelf_obsd armelf
> mips64 elf64btsmip_obsd elf64btsmip
> mips64el elf64ltsmip_obsd elf64ltsmip
> powerpc elf32ppc_obsd elf32ppc
> sparc64 elf64_sparc_obsd elf64_sparc64
>
> Since ld.bfd has a default anyway, it seems easier to leave it unset
> rather than carry a second table plus risk breakage (not that this is
> really widely used; only mupdf, net/utox, www/mozplugger).

Well, it's called LLD_EMUL.

Shouldn't it be just for LLD ?

BTW, LLD_EMUL was never documented. Blame naddy@

Reply | Threaded
Open this post in threaded view
|

Re: arch-defines: also set LLD_EMUL if USE_LLD is set

Stuart Henderson
On 2019/12/04 14:35, Marc Espie wrote:

> On Wed, Dec 04, 2019 at 12:45:46PM +0000, Stuart Henderson wrote:
> > On 2019/12/04 12:17, Marc Espie wrote:
> > > On Wed, Dec 04, 2019 at 10:48:06AM +0000, Stuart Henderson wrote:
> > > > Currently LLD_EMUL is only set for LLD_ARCHS (aarch64 amd64 arm i386).
> > > > A diff to unbreak mupdf build on mips64 requires linking with lld and this
> > > > variable is required. Rather than adding a custom LLD_EMUL definition in
> > > > mupdf let's just make the arch-defines.mk one reachable.
> > > >
> > > > OK?
> > > >
> > > > Index: arch-defines.mk
> > > > ===================================================================
> > > > RCS file: /cvs/ports/infrastructure/mk/arch-defines.mk,v
> > > > retrieving revision 1.69
> > > > diff -u -p -1 -1 -r1.69 arch-defines.mk
> > > > --- arch-defines.mk 9 Nov 2019 15:08:09 -0000 1.69
> > > > +++ arch-defines.mk 4 Dec 2019 10:45:44 -0000
> > > > @@ -57,23 +57,23 @@ DEBUG_PACKAGES =
> > > >  DEBUG_FILES =
> > > >  .endif
> > > >  
> > > >  .if ${PROPERTIES:Mclang}
> > > >  LIBCXX = c++ c++abi pthread
> > > >  LIBECXX = c++ c++abi pthread
> > > >  .else
> > > >  LIBCXX = stdc++ pthread
> > > >  LIBECXX = estdc++>=17 pthread
> > > >  .endif
> > > >  
> > > > -.if ${PROPERTIES:Mlld}
> > > > +.if ${PROPERTIES:Mlld} || defined(USE_LLD) && ${USE_LLD:L} == yes
> > > >  # see llvm/tools/lld/ELF/Driver.cpp
> > > >  .  for A in aarch64.aarch64elf amd64.elf_amd64 arm.armelf i386.elf_i386 \
> > > >              mips64.elf64btsmip mips64el.elf64ltsmip powerpc.elf32ppc \
> > > >              sparc64.elf64_sparc
> > > >  .    if ${MACHINE_ARCH} == ${A:R}
> > > >  LLD_EMUL = -m${A:E}
> > > >  .    endif
> > > >  .  endfor
> > > >  .else
> > > >  LLD_EMUL =
> > > >  .endif
> > > >
> > > Would there be a downside to just always set LLD_EMUL ?
> >
> > It needs to use different strings depending on the linker used:
> >
> > arch ld.bfd ld.lld
> > i386 elf_i386_obsd elf_i386
> > amd64 elf_x86_64_obsd elf_amd64
> > aarch64 n/a aarch64elf
> > arm armelf_obsd armelf
> > mips64 elf64btsmip_obsd elf64btsmip
> > mips64el elf64ltsmip_obsd elf64ltsmip
> > powerpc elf32ppc_obsd elf32ppc
> > sparc64 elf64_sparc_obsd elf64_sparc64
> >
> > Since ld.bfd has a default anyway, it seems easier to leave it unset
> > rather than carry a second table plus risk breakage (not that this is
> > really widely used; only mupdf, net/utox, www/mozplugger).
>
> Well, it's called LLD_EMUL.
>
> Shouldn't it be just for LLD ?

Oh I see what you mean. The way it's currently used ${LLD_EMUL} is
simply added to linker flags (so on an ld.bfd arch it will just be
empty). If LLD_EMUL is always set and uses an LLD-ish value that
won't work with ld.bfd then we'd need to pull in bsd.port.arch.mk
early and add a conditional in every port that uses it. Seems
saner to have a single copy of the messy part in one place.

.include <bsd.port.arch.mk>
.if ${PROPERTIES:Mlld} || defined(USE_LLD) && ${USE_LLD:L} == yes
[...]
.endif

> BTW, LLD_EMUL was never documented. Blame naddy@
>

Reply | Threaded
Open this post in threaded view
|

Re: arch-defines: also set LLD_EMUL if USE_LLD is set

Marc Espie-2
On Wed, Dec 04, 2019 at 04:00:13PM +0000, Stuart Henderson wrote:

> On 2019/12/04 14:35, Marc Espie wrote:
> > On Wed, Dec 04, 2019 at 12:45:46PM +0000, Stuart Henderson wrote:
> > > On 2019/12/04 12:17, Marc Espie wrote:
> > > > On Wed, Dec 04, 2019 at 10:48:06AM +0000, Stuart Henderson wrote:
> > > > > Currently LLD_EMUL is only set for LLD_ARCHS (aarch64 amd64 arm i386).
> > > > > A diff to unbreak mupdf build on mips64 requires linking with lld and this
> > > > > variable is required. Rather than adding a custom LLD_EMUL definition in
> > > > > mupdf let's just make the arch-defines.mk one reachable.
> > > > >
> > > > > OK?
> > > > >
> > > > > Index: arch-defines.mk
> > > > > ===================================================================
> > > > > RCS file: /cvs/ports/infrastructure/mk/arch-defines.mk,v
> > > > > retrieving revision 1.69
> > > > > diff -u -p -1 -1 -r1.69 arch-defines.mk
> > > > > --- arch-defines.mk 9 Nov 2019 15:08:09 -0000 1.69
> > > > > +++ arch-defines.mk 4 Dec 2019 10:45:44 -0000
> > > > > @@ -57,23 +57,23 @@ DEBUG_PACKAGES =
> > > > >  DEBUG_FILES =
> > > > >  .endif
> > > > >  
> > > > >  .if ${PROPERTIES:Mclang}
> > > > >  LIBCXX = c++ c++abi pthread
> > > > >  LIBECXX = c++ c++abi pthread
> > > > >  .else
> > > > >  LIBCXX = stdc++ pthread
> > > > >  LIBECXX = estdc++>=17 pthread
> > > > >  .endif
> > > > >  
> > > > > -.if ${PROPERTIES:Mlld}
> > > > > +.if ${PROPERTIES:Mlld} || defined(USE_LLD) && ${USE_LLD:L} == yes
> > > > >  # see llvm/tools/lld/ELF/Driver.cpp
> > > > >  .  for A in aarch64.aarch64elf amd64.elf_amd64 arm.armelf i386.elf_i386 \
> > > > >              mips64.elf64btsmip mips64el.elf64ltsmip powerpc.elf32ppc \
> > > > >              sparc64.elf64_sparc
> > > > >  .    if ${MACHINE_ARCH} == ${A:R}
> > > > >  LLD_EMUL = -m${A:E}
> > > > >  .    endif
> > > > >  .  endfor
> > > > >  .else
> > > > >  LLD_EMUL =
> > > > >  .endif
> > > > >
> > > > Would there be a downside to just always set LLD_EMUL ?
> > >
> > > It needs to use different strings depending on the linker used:
> > >
> > > arch ld.bfd ld.lld
> > > i386 elf_i386_obsd elf_i386
> > > amd64 elf_x86_64_obsd elf_amd64
> > > aarch64 n/a aarch64elf
> > > arm armelf_obsd armelf
> > > mips64 elf64btsmip_obsd elf64btsmip
> > > mips64el elf64ltsmip_obsd elf64ltsmip
> > > powerpc elf32ppc_obsd elf32ppc
> > > sparc64 elf64_sparc_obsd elf64_sparc64
> > >
> > > Since ld.bfd has a default anyway, it seems easier to leave it unset
> > > rather than carry a second table plus risk breakage (not that this is
> > > really widely used; only mupdf, net/utox, www/mozplugger).
> >
> > Well, it's called LLD_EMUL.
> >
> > Shouldn't it be just for LLD ?
>
> Oh I see what you mean. The way it's currently used ${LLD_EMUL} is
> simply added to linker flags (so on an ld.bfd arch it will just be
> empty). If LLD_EMUL is always set and uses an LLD-ish value that
> won't work with ld.bfd then we'd need to pull in bsd.port.arch.mk
> early and add a conditional in every port that uses it. Seems
> saner to have a single copy of the messy part in one place.
>
> .include <bsd.port.arch.mk>
> .if ${PROPERTIES:Mlld} || defined(USE_LLD) && ${USE_LLD:L} == yes
> [...]
> .endif
>
> > BTW, LLD_EMUL was never documented. Blame naddy@
> >

Oh, the name is 100% misleading then.

Reply | Threaded
Open this post in threaded view
|

Re: arch-defines: also set LLD_EMUL if USE_LLD is set

Stuart Henderson
On 2019/12/04 20:34, Marc Espie wrote:

> On Wed, Dec 04, 2019 at 04:00:13PM +0000, Stuart Henderson wrote:
> > On 2019/12/04 14:35, Marc Espie wrote:
> > > On Wed, Dec 04, 2019 at 12:45:46PM +0000, Stuart Henderson wrote:
> > > > On 2019/12/04 12:17, Marc Espie wrote:
> > > > > On Wed, Dec 04, 2019 at 10:48:06AM +0000, Stuart Henderson wrote:
> > > > > > Currently LLD_EMUL is only set for LLD_ARCHS (aarch64 amd64 arm i386).
> > > > > > A diff to unbreak mupdf build on mips64 requires linking with lld and this
> > > > > > variable is required. Rather than adding a custom LLD_EMUL definition in
> > > > > > mupdf let's just make the arch-defines.mk one reachable.
> > > > > >
> > > > > > OK?
> > > > > >
> > > > > > Index: arch-defines.mk
> > > > > > ===================================================================
> > > > > > RCS file: /cvs/ports/infrastructure/mk/arch-defines.mk,v
> > > > > > retrieving revision 1.69
> > > > > > diff -u -p -1 -1 -r1.69 arch-defines.mk
> > > > > > --- arch-defines.mk 9 Nov 2019 15:08:09 -0000 1.69
> > > > > > +++ arch-defines.mk 4 Dec 2019 10:45:44 -0000
> > > > > > @@ -57,23 +57,23 @@ DEBUG_PACKAGES =
> > > > > >  DEBUG_FILES =
> > > > > >  .endif
> > > > > >  
> > > > > >  .if ${PROPERTIES:Mclang}
> > > > > >  LIBCXX = c++ c++abi pthread
> > > > > >  LIBECXX = c++ c++abi pthread
> > > > > >  .else
> > > > > >  LIBCXX = stdc++ pthread
> > > > > >  LIBECXX = estdc++>=17 pthread
> > > > > >  .endif
> > > > > >  
> > > > > > -.if ${PROPERTIES:Mlld}
> > > > > > +.if ${PROPERTIES:Mlld} || defined(USE_LLD) && ${USE_LLD:L} == yes
> > > > > >  # see llvm/tools/lld/ELF/Driver.cpp
> > > > > >  .  for A in aarch64.aarch64elf amd64.elf_amd64 arm.armelf i386.elf_i386 \
> > > > > >              mips64.elf64btsmip mips64el.elf64ltsmip powerpc.elf32ppc \
> > > > > >              sparc64.elf64_sparc
> > > > > >  .    if ${MACHINE_ARCH} == ${A:R}
> > > > > >  LLD_EMUL = -m${A:E}
> > > > > >  .    endif
> > > > > >  .  endfor
> > > > > >  .else
> > > > > >  LLD_EMUL =
> > > > > >  .endif
> > > > > >
> > > > > Would there be a downside to just always set LLD_EMUL ?
> > > >
> > > > It needs to use different strings depending on the linker used:
> > > >
> > > > arch ld.bfd ld.lld
> > > > i386 elf_i386_obsd elf_i386
> > > > amd64 elf_x86_64_obsd elf_amd64
> > > > aarch64 n/a aarch64elf
> > > > arm armelf_obsd armelf
> > > > mips64 elf64btsmip_obsd elf64btsmip
> > > > mips64el elf64ltsmip_obsd elf64ltsmip
> > > > powerpc elf32ppc_obsd elf32ppc
> > > > sparc64 elf64_sparc_obsd elf64_sparc64
> > > >
> > > > Since ld.bfd has a default anyway, it seems easier to leave it unset
> > > > rather than carry a second table plus risk breakage (not that this is
> > > > really widely used; only mupdf, net/utox, www/mozplugger).
> > >
> > > Well, it's called LLD_EMUL.
> > >
> > > Shouldn't it be just for LLD ?
> >
> > Oh I see what you mean. The way it's currently used ${LLD_EMUL} is
> > simply added to linker flags (so on an ld.bfd arch it will just be
> > empty). If LLD_EMUL is always set and uses an LLD-ish value that
> > won't work with ld.bfd then we'd need to pull in bsd.port.arch.mk
> > early and add a conditional in every port that uses it. Seems
> > saner to have a single copy of the messy part in one place.
> >
> > .include <bsd.port.arch.mk>
> > .if ${PROPERTIES:Mlld} || defined(USE_LLD) && ${USE_LLD:L} == yes
> > [...]
> > .endif
> >
> > > BTW, LLD_EMUL was never documented. Blame naddy@
> > >
>
> Oh, the name is 100% misleading then.
>

I think the description could be something along these lines,

If the linker in use is LLD, LLD_EMUL is defined as a string to set
the usual target emulation type for the build architecture. Otherwise,
LLD_EMUL is empty. Used rarely, for example when linking non-ELF
binary data.

Reply | Threaded
Open this post in threaded view
|

Re: arch-defines: also set LLD_EMUL if USE_LLD is set

Marc Espie-2
On Thu, Dec 05, 2019 at 11:04:16PM +0000, Stuart Henderson wrote:

> On 2019/12/04 20:34, Marc Espie wrote:
> > On Wed, Dec 04, 2019 at 04:00:13PM +0000, Stuart Henderson wrote:
> > > On 2019/12/04 14:35, Marc Espie wrote:
> > > > On Wed, Dec 04, 2019 at 12:45:46PM +0000, Stuart Henderson wrote:
> > > > > On 2019/12/04 12:17, Marc Espie wrote:
> > > > > > On Wed, Dec 04, 2019 at 10:48:06AM +0000, Stuart Henderson wrote:
> > > > > > > Currently LLD_EMUL is only set for LLD_ARCHS (aarch64 amd64 arm i386).
> > > > > > > A diff to unbreak mupdf build on mips64 requires linking with lld and this
> > > > > > > variable is required. Rather than adding a custom LLD_EMUL definition in
> > > > > > > mupdf let's just make the arch-defines.mk one reachable.
> > > > > > >
> > > > > > > OK?
> > > > > > >
> > > > > > > Index: arch-defines.mk
> > > > > > > ===================================================================
> > > > > > > RCS file: /cvs/ports/infrastructure/mk/arch-defines.mk,v
> > > > > > > retrieving revision 1.69
> > > > > > > diff -u -p -1 -1 -r1.69 arch-defines.mk
> > > > > > > --- arch-defines.mk 9 Nov 2019 15:08:09 -0000 1.69
> > > > > > > +++ arch-defines.mk 4 Dec 2019 10:45:44 -0000
> > > > > > > @@ -57,23 +57,23 @@ DEBUG_PACKAGES =
> > > > > > >  DEBUG_FILES =
> > > > > > >  .endif
> > > > > > >  
> > > > > > >  .if ${PROPERTIES:Mclang}
> > > > > > >  LIBCXX = c++ c++abi pthread
> > > > > > >  LIBECXX = c++ c++abi pthread
> > > > > > >  .else
> > > > > > >  LIBCXX = stdc++ pthread
> > > > > > >  LIBECXX = estdc++>=17 pthread
> > > > > > >  .endif
> > > > > > >  
> > > > > > > -.if ${PROPERTIES:Mlld}
> > > > > > > +.if ${PROPERTIES:Mlld} || defined(USE_LLD) && ${USE_LLD:L} == yes
> > > > > > >  # see llvm/tools/lld/ELF/Driver.cpp
> > > > > > >  .  for A in aarch64.aarch64elf amd64.elf_amd64 arm.armelf i386.elf_i386 \
> > > > > > >              mips64.elf64btsmip mips64el.elf64ltsmip powerpc.elf32ppc \
> > > > > > >              sparc64.elf64_sparc
> > > > > > >  .    if ${MACHINE_ARCH} == ${A:R}
> > > > > > >  LLD_EMUL = -m${A:E}
> > > > > > >  .    endif
> > > > > > >  .  endfor
> > > > > > >  .else
> > > > > > >  LLD_EMUL =
> > > > > > >  .endif
> > > > > > >
> > > > > > Would there be a downside to just always set LLD_EMUL ?
> > > > >
> > > > > It needs to use different strings depending on the linker used:
> > > > >
> > > > > arch ld.bfd ld.lld
> > > > > i386 elf_i386_obsd elf_i386
> > > > > amd64 elf_x86_64_obsd elf_amd64
> > > > > aarch64 n/a aarch64elf
> > > > > arm armelf_obsd armelf
> > > > > mips64 elf64btsmip_obsd elf64btsmip
> > > > > mips64el elf64ltsmip_obsd elf64ltsmip
> > > > > powerpc elf32ppc_obsd elf32ppc
> > > > > sparc64 elf64_sparc_obsd elf64_sparc64
> > > > >
> > > > > Since ld.bfd has a default anyway, it seems easier to leave it unset
> > > > > rather than carry a second table plus risk breakage (not that this is
> > > > > really widely used; only mupdf, net/utox, www/mozplugger).
> > > >
> > > > Well, it's called LLD_EMUL.
> > > >
> > > > Shouldn't it be just for LLD ?
> > >
> > > Oh I see what you mean. The way it's currently used ${LLD_EMUL} is
> > > simply added to linker flags (so on an ld.bfd arch it will just be
> > > empty). If LLD_EMUL is always set and uses an LLD-ish value that
> > > won't work with ld.bfd then we'd need to pull in bsd.port.arch.mk
> > > early and add a conditional in every port that uses it. Seems
> > > saner to have a single copy of the messy part in one place.
> > >
> > > .include <bsd.port.arch.mk>
> > > .if ${PROPERTIES:Mlld} || defined(USE_LLD) && ${USE_LLD:L} == yes
> > > [...]
> > > .endif
> > >
> > > > BTW, LLD_EMUL was never documented. Blame naddy@
> > > >
> >
> > Oh, the name is 100% misleading then.
> >
>
> I think the description could be something along these lines,
>
> If the linker in use is LLD, LLD_EMUL is defined as a string to set
> the usual target emulation type for the build architecture. Otherwise,
> LLD_EMUL is empty. Used rarely, for example when linking non-ELF
> binary data.

Well, that belongs in bsd.port.arch.mk(5) not in a random email.