* mips64-linux-gnuabi64 testsuite breakage
@ 2023-04-26 4:53 Alan Modra
2023-04-26 5:46 ` YunQiang Su
0 siblings, 1 reply; 8+ messages in thread
From: Alan Modra @ 2023-04-26 4:53 UTC (permalink / raw)
To: binutils
Cc: YunQiang Su, RichardSandifordrichard.sandiford, Chenghua Xu,
Maciej W. Rozycki
Since 32f1c80375e "MIPS: support mips*64 as CPU and gnuabi64 as ABI",
I'm seeing 187 new testsuite fails on mips64-linux. That strongly
indicates untested, wrong changes have been committed.
--
Alan Modra
Australia Development Lab, IBM
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: mips64-linux-gnuabi64 testsuite breakage
2023-04-26 4:53 mips64-linux-gnuabi64 testsuite breakage Alan Modra
@ 2023-04-26 5:46 ` YunQiang Su
2023-05-02 19:33 ` Maciej W. Rozycki
0 siblings, 1 reply; 8+ messages in thread
From: YunQiang Su @ 2023-04-26 5:46 UTC (permalink / raw)
To: Alan Modra
Cc: binutils, YunQiang Su, RichardSandifordrichard.sandiford,
Chenghua Xu, Maciej W. Rozycki
Alan Modra via Binutils <binutils@sourceware.org> 于2023年4月26日周三 12:53写道:
>
> Since 32f1c80375e "MIPS: support mips*64 as CPU and gnuabi64 as ABI",
I will fix the test suite.
The reason is that: the previous test suite assumes the default ABI is
n32 even the triple
with -gnuabi64.
> I'm seeing 187 new testsuite fails on mips64-linux. That strongly
> indicates untested, wrong changes have been committed.
>
> --
> Alan Modra
> Australia Development Lab, IBM
--
YunQiang Su
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: mips64-linux-gnuabi64 testsuite breakage
2023-04-26 5:46 ` YunQiang Su
@ 2023-05-02 19:33 ` Maciej W. Rozycki
2023-05-04 1:55 ` YunQiang Su
2023-05-09 8:13 ` Richard Sandiford
0 siblings, 2 replies; 8+ messages in thread
From: Maciej W. Rozycki @ 2023-05-02 19:33 UTC (permalink / raw)
To: YunQiang Su
Cc: Alan Modra, binutils, YunQiang Su,
RichardSandifordrichard.sandiford, Chenghua Xu
On Wed, 26 Apr 2023, YunQiang Su wrote:
> > Since 32f1c80375e "MIPS: support mips*64 as CPU and gnuabi64 as ABI",
>
> I will fix the test suite.
> The reason is that: the previous test suite assumes the default ABI is
> n32 even the triple
> with -gnuabi64.
I can see extra 3346 regressions across various MIPS targets compared to
a checkout from Jan. Including ones like (parts of log trimmed for
clarity):
../as-new --defsym count=960 /scratch/vol1/binutils/binutils-mips-test/binutils-src/gas/testsuite/gas/mips/branch-swap-2.s
gas/testsuite/gas/mips/branch-swap-2.s:1: Fatal error: `micromips' cannot be used with `mips32r6'
FAIL: MIPS branch swapping (960)
for `mips-img-elf' which indicates that the default architecture has
changed for GAS for this target. I think this is due to your commit
9171de358f23 ("MIPS: default output r6 obj if the triple is r6"), which
has this part among others:
+ # If Vendor is IMG, then MIPSr6 is used
+ case ${target} in
+ mips*64*-img-*)
+ mips_cpu=mips64r6
+ ;;
+ mips*-img-*)
+ mips_cpu=mips32r6
+ ;;
+ esac
Not only this change is wrong (you can't just arbitrarily change a
configuration that has been in the wild for ~8.5 years as it'll break
things for people who rely on the established semantics), but it hasn't
been properly verified either, as a change is supposed not to cause
regressions in the testsuite.
Please revert the part quoted and fix the regressions. Thank you.
Maciej
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: mips64-linux-gnuabi64 testsuite breakage
2023-05-02 19:33 ` Maciej W. Rozycki
@ 2023-05-04 1:55 ` YunQiang Su
2023-05-07 17:46 ` Maciej W. Rozycki
2023-05-09 8:13 ` Richard Sandiford
1 sibling, 1 reply; 8+ messages in thread
From: YunQiang Su @ 2023-05-04 1:55 UTC (permalink / raw)
To: Maciej W. Rozycki
Cc: Alan Modra, binutils, YunQiang Su,
RichardSandifordrichard.sandiford, Chenghua Xu
Maciej W. Rozycki <macro@orcam.me.uk> 于2023年5月3日周三 03:33写道:
>
> On Wed, 26 Apr 2023, YunQiang Su wrote:
>
> > > Since 32f1c80375e "MIPS: support mips*64 as CPU and gnuabi64 as ABI",
> >
> > I will fix the test suite.
> > The reason is that: the previous test suite assumes the default ABI is
> > n32 even the triple
> > with -gnuabi64.
>
> I can see extra 3346 regressions across various MIPS targets compared to
> a checkout from Jan. Including ones like (parts of log trimmed for
> clarity):
>
> ../as-new --defsym count=960 /scratch/vol1/binutils/binutils-mips-test/binutils-src/gas/testsuite/gas/mips/branch-swap-2.s
> gas/testsuite/gas/mips/branch-swap-2.s:1: Fatal error: `micromips' cannot be used with `mips32r6'
> FAIL: MIPS branch swapping (960)
>
> for `mips-img-elf' which indicates that the default architecture has
> changed for GAS for this target. I think this is due to your commit
> 9171de358f23 ("MIPS: default output r6 obj if the triple is r6"), which
> has this part among others:
>
> + # If Vendor is IMG, then MIPSr6 is used
> + case ${target} in
> + mips*64*-img-*)
> + mips_cpu=mips64r6
> + ;;
> + mips*-img-*)
> + mips_cpu=mips32r6
> + ;;
> + esac
>
> Not only this change is wrong (you can't just arbitrarily change a
> configuration that has been in the wild for ~8.5 years as it'll break
> things for people who rely on the established semantics), but it hasn't
> been properly verified either, as a change is supposed not to cause
> regressions in the testsuite.
>
> Please revert the part quoted and fix the regressions. Thank you.
Reverted.
And I am working on fix the tesutsuite for `-gnuabi64` and `mipsisaXXr6`.
>
> Maciej
--
YunQiang Su
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: mips64-linux-gnuabi64 testsuite breakage
2023-05-04 1:55 ` YunQiang Su
@ 2023-05-07 17:46 ` Maciej W. Rozycki
2023-05-09 3:15 ` YunQiang Su
0 siblings, 1 reply; 8+ messages in thread
From: Maciej W. Rozycki @ 2023-05-07 17:46 UTC (permalink / raw)
To: YunQiang Su
Cc: Alan Modra, binutils, YunQiang Su,
RichardSandifordrichard.sandiford, Chenghua Xu
On Thu, 4 May 2023, YunQiang Su wrote:
> > + # If Vendor is IMG, then MIPSr6 is used
> > + case ${target} in
> > + mips*64*-img-*)
> > + mips_cpu=mips64r6
> > + ;;
> > + mips*-img-*)
> > + mips_cpu=mips32r6
> > + ;;
> > + esac
> >
> > Not only this change is wrong (you can't just arbitrarily change a
> > configuration that has been in the wild for ~8.5 years as it'll break
> > things for people who rely on the established semantics), but it hasn't
> > been properly verified either, as a change is supposed not to cause
> > regressions in the testsuite.
> >
> > Please revert the part quoted and fix the regressions. Thank you.
>
> Reverted.
Thank you.
FAOD if you think a triplet that implicitly defaults to R6 is useful,
then feel free to submit a change for say `mips*-cipunited-*' to do so (or
maybe `mips*-cip-*' if you want to keep it short and the company policy
allows using such a short name), as this would be a configuration that had
no previous mention/semantics in our tree and therefore I would have no
concerns about it.
Overall I'd suggest the `--with-arch=' approach previously discussed and
already implemented for MIPS targets in GCC and also some other targets in
binutils. It is more generic, although it may complicate testing, so it
may have to be deployed with care.
> And I am working on fix the tesutsuite for `-gnuabi64` and `mipsisaXXr6`.
I look forward to your fixes.
Maciej
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: mips64-linux-gnuabi64 testsuite breakage
2023-05-07 17:46 ` Maciej W. Rozycki
@ 2023-05-09 3:15 ` YunQiang Su
0 siblings, 0 replies; 8+ messages in thread
From: YunQiang Su @ 2023-05-09 3:15 UTC (permalink / raw)
To: Maciej W. Rozycki
Cc: Alan Modra, binutils, YunQiang Su,
RichardSandifordrichard.sandiford, Chenghua Xu
Maciej W. Rozycki <macro@orcam.me.uk> 于2023年5月8日周一 01:46写道:
>
> On Thu, 4 May 2023, YunQiang Su wrote:
>
> > > + # If Vendor is IMG, then MIPSr6 is used
> > > + case ${target} in
> > > + mips*64*-img-*)
> > > + mips_cpu=mips64r6
> > > + ;;
> > > + mips*-img-*)
> > > + mips_cpu=mips32r6
> > > + ;;
> > > + esac
> > >
> > > Not only this change is wrong (you can't just arbitrarily change a
> > > configuration that has been in the wild for ~8.5 years as it'll break
> > > things for people who rely on the established semantics), but it hasn't
> > > been properly verified either, as a change is supposed not to cause
> > > regressions in the testsuite.
> > >
> > > Please revert the part quoted and fix the regressions. Thank you.
> >
> > Reverted.
>
> Thank you.
>
> FAOD if you think a triplet that implicitly defaults to R6 is useful,
No, I don't think that it is useful at all. It is so uglyyyyyyy.
In fact, we should use the correct cpu name in triple, just like
mipsisa32r6 etc.
> then feel free to submit a change for say `mips*-cipunited-*' to do so (or
> maybe `mips*-cip-*' if you want to keep it short and the company policy
> allows using such a short name), as this would be a configuration that had
> no previous mention/semantics in our tree and therefore I would have no
> concerns about it.
>
No, I won't. I submit the patch about `img`, is due to that, this vendor value
did/does/being used.
IMG/MIPS/Wave/MIPS does provide the toolchain with img as its vendor name,
and the default arch is R6.
https://codescape.mips.com/components/toolchain/2021.09-01/downloads.html
> Overall I'd suggest the `--with-arch=' approach previously discussed and
> already implemented for MIPS targets in GCC and also some other targets in
> binutils. It is more generic, although it may complicate testing, so it
> may have to be deployed with care.
>
> > And I am working on fix the tesutsuite for `-gnuabi64` and `mipsisaXXr6`.
>
> I look forward to your fixes.
>
> Maciej
--
YunQiang Su
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: mips64-linux-gnuabi64 testsuite breakage
2023-05-02 19:33 ` Maciej W. Rozycki
2023-05-04 1:55 ` YunQiang Su
@ 2023-05-09 8:13 ` Richard Sandiford
2023-05-18 15:58 ` Maciej W. Rozycki
1 sibling, 1 reply; 8+ messages in thread
From: Richard Sandiford @ 2023-05-09 8:13 UTC (permalink / raw)
To: Maciej W. Rozycki
Cc: YunQiang Su, Alan Modra, binutils, YunQiang Su, Chenghua Xu
"Maciej W. Rozycki" <macro@orcam.me.uk> writes:
> On Wed, 26 Apr 2023, YunQiang Su wrote:
>
>> > Since 32f1c80375e "MIPS: support mips*64 as CPU and gnuabi64 as ABI",
>>
>> I will fix the test suite.
>> The reason is that: the previous test suite assumes the default ABI is
>> n32 even the triple
>> with -gnuabi64.
>
> I can see extra 3346 regressions across various MIPS targets compared to
> a checkout from Jan. Including ones like (parts of log trimmed for
> clarity):
>
> ../as-new --defsym count=960 /scratch/vol1/binutils/binutils-mips-test/binutils-src/gas/testsuite/gas/mips/branch-swap-2.s
> gas/testsuite/gas/mips/branch-swap-2.s:1: Fatal error: `micromips' cannot be used with `mips32r6'
> FAIL: MIPS branch swapping (960)
>
> for `mips-img-elf' which indicates that the default architecture has
> changed for GAS for this target. I think this is due to your commit
> 9171de358f23 ("MIPS: default output r6 obj if the triple is r6"), which
> has this part among others:
>
> + # If Vendor is IMG, then MIPSr6 is used
> + case ${target} in
> + mips*64*-img-*)
> + mips_cpu=mips64r6
> + ;;
> + mips*-img-*)
> + mips_cpu=mips32r6
> + ;;
> + esac
>
> Not only this change is wrong (you can't just arbitrarily change a
> configuration that has been in the wild for ~8.5 years as it'll break
> things for people who rely on the established semantics),
It wasn't supposed to be an arbitrary change, but instead was supposed
to sync GAS's default to GCC's. The GCC img toolchain has been an r6
toolchain since it was added in 2014. The fact that GAS instead defaults
to mips1/mips3 seems like a bug, since mips1 isn't link-compatible with
mips32r6 and mips3 isn't link-compatible with mips64r6.
> but it hasn't been properly verified either, as a change is supposed
> not to cause regressions in the testsuite.
Can't argue with that part. But...
> Please revert the part quoted and fix the regressions. Thank you.
...I think it does make sense to resurrect the patch in a testsuite-friendly
form.
Thanks,
Richard
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: mips64-linux-gnuabi64 testsuite breakage
2023-05-09 8:13 ` Richard Sandiford
@ 2023-05-18 15:58 ` Maciej W. Rozycki
0 siblings, 0 replies; 8+ messages in thread
From: Maciej W. Rozycki @ 2023-05-18 15:58 UTC (permalink / raw)
To: Richard Sandiford
Cc: YunQiang Su, Alan Modra, binutils, YunQiang Su, Chenghua Xu
On Tue, 9 May 2023, Richard Sandiford wrote:
> > Not only this change is wrong (you can't just arbitrarily change a
> > configuration that has been in the wild for ~8.5 years as it'll break
> > things for people who rely on the established semantics),
>
> It wasn't supposed to be an arbitrary change, but instead was supposed
> to sync GAS's default to GCC's. The GCC img toolchain has been an r6
> toolchain since it was added in 2014. The fact that GAS instead defaults
> to mips1/mips3 seems like a bug, since mips1 isn't link-compatible with
> mips32r6 and mips3 isn't link-compatible with mips64r6.
Fair enough, but:
1. It should have been submitted as a distinct patch on its own, rather
than bundled with an unrelated change.
2. The rationale as you quote it should have been given in the change
description.
> > Please revert the part quoted and fix the regressions. Thank you.
>
> ...I think it does make sense to resurrect the patch in a testsuite-friendly
> form.
And following the patch submission requirements repeated above, right.
Maciej
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2023-05-18 15:58 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-04-26 4:53 mips64-linux-gnuabi64 testsuite breakage Alan Modra
2023-04-26 5:46 ` YunQiang Su
2023-05-02 19:33 ` Maciej W. Rozycki
2023-05-04 1:55 ` YunQiang Su
2023-05-07 17:46 ` Maciej W. Rozycki
2023-05-09 3:15 ` YunQiang Su
2023-05-09 8:13 ` Richard Sandiford
2023-05-18 15:58 ` Maciej W. Rozycki
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).