Re: riscv64: ld.lld is too picky on ABI mismatch

[Mark Kettenis]

> From: Jeremie Courreges-Anglas 
> Cc: Mark Kettenis , Patrick Wildt 
> Date: Thu, 10 Mar 2022 02:03:10 +0100
> Content-Type: text/plain
> 
> On Mon, Oct 25 2021, Jeremie Courreges-Anglas  wrote:
> > On Mon, Oct 25 2021, Patrick Wildt  wrote:
> >> Am Mon, Oct 25, 2021 at 11:43:55AM +0200 schrieb Mark Kettenis:
> >>> > From: Jeremie Courreges-Anglas 
> >>> > Date: Sun, 24 Oct 2021 17:30:46 +0100
> >>> > 
> >>> > clang(1) defaults to FP ABI but ld(1) -melf64lriscv doesn't.  This is
> >>> > a problem as soon as a port tries to use raw ld(1) -b binary to embed
> >>> > data in a .o file.
> >>> > 
> >>> > Downgrading this hard error to a warning seems more useful as far as the
> >>> > ports tree is concerned.  The diff below fixes
> >>> > databases/postgresql-pllua and should also fix textproc/mupdf and 
> >>> > net/utox.
> >>> > 
> >>> > There's another diff here: https://reviews.llvm.org/D106378
> >>> > but it's slightly more complicated and it received seemingly negative
> >>> > feedback.  So I'm just using a minimal change to fit our needs.
> >>> > 
> >>> > ok?
> >>> 
> >>> I think we should try to avoid deviating from upstream as much as
> >>> possible.  And I agree with the folks who argue that the mismatching
> >>> objects are created with the wrong tools.
> >
> > Well the only alternative they suggest is using assembly and .incbin.
> > Maybe that works for the FreeBSD folks working on low-level stuff, but
> > not so much as a general purpose tool to include binary data in builds.
> >
> >>> But if mortimer@ and
> >>> patrick@ can deal with carrying this local modification I won't
> >>> object.
> >
> > I can't speak for them but given the local changes we have in clang and
> > lld land this two lines diff doesn't seem daunting. ;)
> >
> >> Well, I was about to send an LLVM 13 diff... so can we revisit this diff
> >> when we're updated to LLVM 13?
> 
> I have added comments on https://reviews.llvm.org/D106378#3219454 but
> the discussion has stalled again (I'll try to revive it).
> 
> I'm still using the diff below on the riscv64 ports build machines, and
> I'd like to get rid of the burden of keeping it in my tree.
> 
> ok?

ok kettenis@

> Index: ELF/Arch/RISCV.cpp
> ===
> RCS file: /cvs/src/gnu/llvm/lld/ELF/Arch/RISCV.cpp,v
> retrieving revision 1.1.1.3
> diff -u -p -r1.1.1.3 RISCV.cpp
> --- ELF/Arch/RISCV.cpp17 Dec 2021 12:25:02 -  1.1.1.3
> +++ ELF/Arch/RISCV.cpp28 Jan 2022 09:11:18 -
> @@ -124,8 +124,8 @@ uint32_t RISCV::calcEFlags() const {
>target |= EF_RISCV_RVC;
>  
>  if ((eflags & EF_RISCV_FLOAT_ABI) != (target & EF_RISCV_FLOAT_ABI))
> -  error(toString(f) +
> -": cannot link object files with different floating-point ABI");
> +  warn(toString(f) +
> +": linking object files with different floating-point ABI");
>  
>  if ((eflags & EF_RISCV_RVE) != (target & EF_RISCV_RVE))
>error(toString(f) +
> 
> 
> -- 
> jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF  DDCC 0DFA 74AE 1524 E7EE
> 

Re: riscv64: ld.lld is too picky on ABI mismatch

[Jeremie Courreges-Anglas]

On Mon, Oct 25 2021, Jeremie Courreges-Anglas  wrote:
> On Mon, Oct 25 2021, Patrick Wildt  wrote:
>> Am Mon, Oct 25, 2021 at 11:43:55AM +0200 schrieb Mark Kettenis:
>>> > From: Jeremie Courreges-Anglas 
>>> > Date: Sun, 24 Oct 2021 17:30:46 +0100
>>> > 
>>> > clang(1) defaults to FP ABI but ld(1) -melf64lriscv doesn't.  This is
>>> > a problem as soon as a port tries to use raw ld(1) -b binary to embed
>>> > data in a .o file.
>>> > 
>>> > Downgrading this hard error to a warning seems more useful as far as the
>>> > ports tree is concerned.  The diff below fixes
>>> > databases/postgresql-pllua and should also fix textproc/mupdf and 
>>> > net/utox.
>>> > 
>>> > There's another diff here: https://reviews.llvm.org/D106378
>>> > but it's slightly more complicated and it received seemingly negative
>>> > feedback.  So I'm just using a minimal change to fit our needs.
>>> > 
>>> > ok?
>>> 
>>> I think we should try to avoid deviating from upstream as much as
>>> possible.  And I agree with the folks who argue that the mismatching
>>> objects are created with the wrong tools.
>
> Well the only alternative they suggest is using assembly and .incbin.
> Maybe that works for the FreeBSD folks working on low-level stuff, but
> not so much as a general purpose tool to include binary data in builds.
>
>>> But if mortimer@ and
>>> patrick@ can deal with carrying this local modification I won't
>>> object.
>
> I can't speak for them but given the local changes we have in clang and
> lld land this two lines diff doesn't seem daunting. ;)
>
>> Well, I was about to send an LLVM 13 diff... so can we revisit this diff
>> when we're updated to LLVM 13?

I have added comments on https://reviews.llvm.org/D106378#3219454 but
the discussion has stalled again (I'll try to revive it).

I'm still using the diff below on the riscv64 ports build machines, and
I'd like to get rid of the burden of keeping it in my tree.

ok?


Index: ELF/Arch/RISCV.cpp
===
RCS file: /cvs/src/gnu/llvm/lld/ELF/Arch/RISCV.cpp,v
retrieving revision 1.1.1.3
diff -u -p -r1.1.1.3 RISCV.cpp
--- ELF/Arch/RISCV.cpp  17 Dec 2021 12:25:02 -  1.1.1.3
+++ ELF/Arch/RISCV.cpp  28 Jan 2022 09:11:18 -
@@ -124,8 +124,8 @@ uint32_t RISCV::calcEFlags() const {
   target |= EF_RISCV_RVC;
 
 if ((eflags & EF_RISCV_FLOAT_ABI) != (target & EF_RISCV_FLOAT_ABI))
-  error(toString(f) +
-": cannot link object files with different floating-point ABI");
+  warn(toString(f) +
+": linking object files with different floating-point ABI");
 
 if ((eflags & EF_RISCV_RVE) != (target & EF_RISCV_RVE))
   error(toString(f) +


-- 
jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF  DDCC 0DFA 74AE 1524 E7EE

Re: riscv64: ld.lld is too picky on ABI mismatch

[Jeremie Courreges-Anglas]

On Mon, Oct 25 2021, Patrick Wildt  wrote:
> Am Mon, Oct 25, 2021 at 11:43:55AM +0200 schrieb Mark Kettenis:
>> > From: Jeremie Courreges-Anglas 
>> > Date: Sun, 24 Oct 2021 17:30:46 +0100
>> > 
>> > clang(1) defaults to FP ABI but ld(1) -melf64lriscv doesn't.  This is
>> > a problem as soon as a port tries to use raw ld(1) -b binary to embed
>> > data in a .o file.
>> > 
>> > Downgrading this hard error to a warning seems more useful as far as the
>> > ports tree is concerned.  The diff below fixes
>> > databases/postgresql-pllua and should also fix textproc/mupdf and net/utox.
>> > 
>> > There's another diff here: https://reviews.llvm.org/D106378
>> > but it's slightly more complicated and it received seemingly negative
>> > feedback.  So I'm just using a minimal change to fit our needs.
>> > 
>> > ok?
>> 
>> I think we should try to avoid deviating from upstream as much as
>> possible.  And I agree with the folks who argue that the mismatching
>> objects are created with the wrong tools.

Well the only alternative they suggest is using assembly and .incbin.
Maybe that works for the FreeBSD folks working on low-level stuff, but
not so much as a general purpose tool to include binary data in builds.

>> But if mortimer@ and
>> patrick@ can deal with carrying this local modification I won't
>> object.

I can't speak for them but given the local changes we have in clang and
lld land this two lines diff doesn't seem daunting. ;)

> Well, I was about to send an LLVM 13 diff... so can we revisit this diff
> when we're updated to LLVM 13?

Sure!

>> > Index: ELF/Arch/RISCV.cpp
>> > ===
>> > RCS file: /cvs/src/gnu/llvm/lld/ELF/Arch/RISCV.cpp,v
>> > retrieving revision 1.1.1.2
>> > diff -u -p -r1.1.1.2 RISCV.cpp
>> > --- ELF/Arch/RISCV.cpp 28 Apr 2021 12:29:31 -  1.1.1.2
>> > +++ ELF/Arch/RISCV.cpp 24 Oct 2021 15:55:49 -
>> > @@ -122,8 +122,8 @@ uint32_t RISCV::calcEFlags() const {
>> >target |= EF_RISCV_RVC;
>> >  
>> >  if ((eflags & EF_RISCV_FLOAT_ABI) != (target & EF_RISCV_FLOAT_ABI))
>> > -  error(toString(f) +
>> > -": cannot link object files with different floating-point 
>> > ABI");
>> > +  warn(toString(f) +
>> > +": linking object files with different floating-point ABI");
>> >  
>> >  if ((eflags & EF_RISCV_RVE) != (target & EF_RISCV_RVE))
>> >error(toString(f) +
>> > 
>> > 
>> > -- 
>> > jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF  DDCC 0DFA 74AE 1524 E7EE
>> > 
>> > 
>> 
>

-- 
jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF  DDCC 0DFA 74AE 1524 E7EE

Re: riscv64: ld.lld is too picky on ABI mismatch

[Patrick Wildt]

Am Mon, Oct 25, 2021 at 11:43:55AM +0200 schrieb Mark Kettenis:
> > From: Jeremie Courreges-Anglas 
> > Date: Sun, 24 Oct 2021 17:30:46 +0100
> > 
> > clang(1) defaults to FP ABI but ld(1) -melf64lriscv doesn't.  This is
> > a problem as soon as a port tries to use raw ld(1) -b binary to embed
> > data in a .o file.
> > 
> > Downgrading this hard error to a warning seems more useful as far as the
> > ports tree is concerned.  The diff below fixes
> > databases/postgresql-pllua and should also fix textproc/mupdf and net/utox.
> > 
> > There's another diff here: https://reviews.llvm.org/D106378
> > but it's slightly more complicated and it received seemingly negative
> > feedback.  So I'm just using a minimal change to fit our needs.
> > 
> > ok?
> 
> I think we should try to avoid deviating from upstream as much as
> possible.  And I agree with the folks who argue that the mismatching
> objects are created with the wrong tools.  But if mortimer@ and
> patrick@ can deal with carrying this local modification I won't
> object.

Well, I was about to send an LLVM 13 diff... so can we revisit this diff
when we're updated to LLVM 13?

> > Index: ELF/Arch/RISCV.cpp
> > ===
> > RCS file: /cvs/src/gnu/llvm/lld/ELF/Arch/RISCV.cpp,v
> > retrieving revision 1.1.1.2
> > diff -u -p -r1.1.1.2 RISCV.cpp
> > --- ELF/Arch/RISCV.cpp  28 Apr 2021 12:29:31 -  1.1.1.2
> > +++ ELF/Arch/RISCV.cpp  24 Oct 2021 15:55:49 -
> > @@ -122,8 +122,8 @@ uint32_t RISCV::calcEFlags() const {
> >target |= EF_RISCV_RVC;
> >  
> >  if ((eflags & EF_RISCV_FLOAT_ABI) != (target & EF_RISCV_FLOAT_ABI))
> > -  error(toString(f) +
> > -": cannot link object files with different floating-point 
> > ABI");
> > +  warn(toString(f) +
> > +": linking object files with different floating-point ABI");
> >  
> >  if ((eflags & EF_RISCV_RVE) != (target & EF_RISCV_RVE))
> >error(toString(f) +
> > 
> > 
> > -- 
> > jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF  DDCC 0DFA 74AE 1524 E7EE
> > 
> > 
> 

Re: riscv64: ld.lld is too picky on ABI mismatch

[Mark Kettenis]

> From: Jeremie Courreges-Anglas 
> Date: Sun, 24 Oct 2021 17:30:46 +0100
> 
> clang(1) defaults to FP ABI but ld(1) -melf64lriscv doesn't.  This is
> a problem as soon as a port tries to use raw ld(1) -b binary to embed
> data in a .o file.
> 
> Downgrading this hard error to a warning seems more useful as far as the
> ports tree is concerned.  The diff below fixes
> databases/postgresql-pllua and should also fix textproc/mupdf and net/utox.
> 
> There's another diff here: https://reviews.llvm.org/D106378
> but it's slightly more complicated and it received seemingly negative
> feedback.  So I'm just using a minimal change to fit our needs.
> 
> ok?

I think we should try to avoid deviating from upstream as much as
possible.  And I agree with the folks who argue that the mismatching
objects are created with the wrong tools.  But if mortimer@ and
patrick@ can deal with carrying this local modification I won't
object.

> Index: ELF/Arch/RISCV.cpp
> ===
> RCS file: /cvs/src/gnu/llvm/lld/ELF/Arch/RISCV.cpp,v
> retrieving revision 1.1.1.2
> diff -u -p -r1.1.1.2 RISCV.cpp
> --- ELF/Arch/RISCV.cpp28 Apr 2021 12:29:31 -  1.1.1.2
> +++ ELF/Arch/RISCV.cpp24 Oct 2021 15:55:49 -
> @@ -122,8 +122,8 @@ uint32_t RISCV::calcEFlags() const {
>target |= EF_RISCV_RVC;
>  
>  if ((eflags & EF_RISCV_FLOAT_ABI) != (target & EF_RISCV_FLOAT_ABI))
> -  error(toString(f) +
> -": cannot link object files with different floating-point ABI");
> +  warn(toString(f) +
> +": linking object files with different floating-point ABI");
>  
>  if ((eflags & EF_RISCV_RVE) != (target & EF_RISCV_RVE))
>error(toString(f) +
> 
> 
> -- 
> jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF  DDCC 0DFA 74AE 1524 E7EE
> 
> 

riscv64: ld.lld is too picky on ABI mismatch

[Jeremie Courreges-Anglas]

clang(1) defaults to FP ABI but ld(1) -melf64lriscv doesn't.  This is
a problem as soon as a port tries to use raw ld(1) -b binary to embed
data in a .o file.

Downgrading this hard error to a warning seems more useful as far as the
ports tree is concerned.  The diff below fixes
databases/postgresql-pllua and should also fix textproc/mupdf and net/utox.

There's another diff here: https://reviews.llvm.org/D106378
but it's slightly more complicated and it received seemingly negative
feedback.  So I'm just using a minimal change to fit our needs.

ok?


Index: ELF/Arch/RISCV.cpp
===
RCS file: /cvs/src/gnu/llvm/lld/ELF/Arch/RISCV.cpp,v
retrieving revision 1.1.1.2
diff -u -p -r1.1.1.2 RISCV.cpp
--- ELF/Arch/RISCV.cpp  28 Apr 2021 12:29:31 -  1.1.1.2
+++ ELF/Arch/RISCV.cpp  24 Oct 2021 15:55:49 -
@@ -122,8 +122,8 @@ uint32_t RISCV::calcEFlags() const {
   target |= EF_RISCV_RVC;
 
 if ((eflags & EF_RISCV_FLOAT_ABI) != (target & EF_RISCV_FLOAT_ABI))
-  error(toString(f) +
-": cannot link object files with different floating-point ABI");
+  warn(toString(f) +
+": linking object files with different floating-point ABI");
 
 if ((eflags & EF_RISCV_RVE) != (target & EF_RISCV_RVE))
   error(toString(f) +


-- 
jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF  DDCC 0DFA 74AE 1524 E7EE