Testsuite failures on sparc64 due to unaligned access since 1.5.0

Testsuite failures on sparc64 due to unaligned access since 1.5.0

[John Paul Adrian Glaubitz]

Hello!

The testsuite has been failing on sparc64 since around 1.5.0 due to unaligned access:

dtc -I dts -O dts -o type-preservation.dts.test.dts type-preservation.dts:	FAIL Killed by SIGBUS
dtc -I dts -O dts type-preservation.dts.test.dts:	FAIL Returned error code 1
cmp type-preservation.dts type-preservation.dts.test.dts:	FAIL Returned error code 1
dtc -I dts -O dtb -o path-references.test.dtb path-references.dts:	PASS
dtc -I dts -O dts -o path-references.test.dts path-references.dts:	PASS
dtc -I dts -O dtb -o path-references.test.dts.test.dtb path-references.test.dts:	PASS
dtbs_equal_ordered path-references.test.dtb path-references.test.dts.test.dtb:	PASS
dtc -I dts -O yaml -o type-preservation.test.dt.yaml type-preservation.dts:	FAIL Killed by SIGBUS

I have not done any bisecting yet, but this is most likely a result of someone casting
a pointer of type A into a pointer of type B which is normally not allowed but unfortunately
also not something that gcc complains about.

Full build log can be found in [1]. Access to a sparc64 porterbox running Linux can be obtained
through the gcc compile farm [2].

Thanks,
Adrian

> [1] https://buildd.debian.org/status/fetch.php?pkg=device-tree-compiler&arch=sparc64&ver=1.5.0-2&stamp=1565897062&raw=0
> [2] https://gcc.gnu.org/wiki/CompileFarm

-- 
 .''`.  John Paul Adrian Glaubitz
: :' :  Debian Developer - glaubitz-8fiUuRrzOP0dnm+yROfE0A@public.gmane.org
`. `'   Freie Universitaet Berlin - glaubitz-1Olz3AKvcsuAKZTfuerNgRvVK+yQ3ZXh@public.gmane.org
  `-    GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913

Re: Testsuite failures on sparc64 due to unaligned access since 1.5.0

[John Paul Adrian Glaubitz]

On 12/10/20 10:01 AM, John Paul Adrian Glaubitz wrote:
> I have not done any bisecting yet, but this is most likely a result of someone casting
> a pointer of type A into a pointer of type B which is normally not allowed but unfortunately
> also not something that gcc complains about.

Hmm, it seems someone fixed the issue on git master already:

Ran 36 tests in 0.017s

OK
********** TEST SUMMARY
*     Total testcases:  2068
*                PASS:  2068
*                FAIL:  0
*   Bad configuration:  0
* Strange test result:  0
**********

Adrian

-- 
 .''`.  John Paul Adrian Glaubitz
: :' :  Debian Developer - glaubitz-8fiUuRrzOP0dnm+yROfE0A@public.gmane.org
`. `'   Freie Universitaet Berlin - glaubitz-1Olz3AKvcsuAKZTfuerNgRvVK+yQ3ZXh@public.gmane.org
  `-    GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913

Re: Testsuite failures on sparc64 due to unaligned access since 1.5.0

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 1072 bytes --]

On Thu, Dec 10, 2020 at 10:04:47AM +0100, John Paul Adrian Glaubitz wrote:
> On 12/10/20 10:01 AM, John Paul Adrian Glaubitz wrote:
> > I have not done any bisecting yet, but this is most likely a result of someone casting
> > a pointer of type A into a pointer of type B which is normally not allowed but unfortunately
> > also not something that gcc complains about.
> 
> Hmm, it seems someone fixed the issue on git master already:

Right, alignment issues have been a perennial problem.  We find some
platform that has alignment constraints that break, fix that, then
someone finds the alignment fixups cause horrible performance
problems, repeat ad nauseum.

There's been another round of this pretty recently, which is probably
the cause of the fixup.  I just hope the re-optimizations that Tom
Rini has in his pipeline don't break things again for you.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: Testsuite failures on sparc64 due to unaligned access since 1.5.0

[Rob Herring]

On Thu, Dec 10, 2020 at 7:09 PM David Gibson
<david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org> wrote:
>
> On Thu, Dec 10, 2020 at 10:04:47AM +0100, John Paul Adrian Glaubitz wrote:
> > On 12/10/20 10:01 AM, John Paul Adrian Glaubitz wrote:
> > > I have not done any bisecting yet, but this is most likely a result of someone casting
> > > a pointer of type A into a pointer of type B which is normally not allowed but unfortunately
> > > also not something that gcc complains about.
> >
> > Hmm, it seems someone fixed the issue on git master already:
>
> Right, alignment issues have been a perennial problem.  We find some
> platform that has alignment constraints that break, fix that, then
> someone finds the alignment fixups cause horrible performance
> problems, repeat ad nauseum.
>
> There's been another round of this pretty recently, which is probably
> the cause of the fixup.  I just hope the re-optimizations that Tom
> Rini has in his pipeline don't break things again for you.

I think the case here is dtc, not libfdt. My guess is something to do
with marker handling writing out dts.

Rob

Re: Testsuite failures on sparc64 due to unaligned access since 1.5.0

[John Paul Adrian Glaubitz]

Hi!

On 12/11/20 2:07 AM, David Gibson wrote:
> Right, alignment issues have been a perennial problem.  We find some
> platform that has alignment constraints that break, fix that, then
> someone finds the alignment fixups cause horrible performance
> problems, repeat ad nauseum.

From my discussions with GCC developers I have learnt that it shouldn't
be a problem to use memcpy() in such cases to avoid aliasing problems
resulting in unaligned access.

GCC will just optimize memcpy() in cases where an unaligned access is
faster.

> There's been another round of this pretty recently, which is probably
> the cause of the fixup.  I just hope the re-optimizations that Tom
> Rini has in his pipeline don't break things again for you.

The GCC compile farm has two fast SPARC servers [1] running Debian unstable,
so I suggest testing such changes there before merging them.

Adrian

> [1] https://gcc.gnu.org/wiki/CompileFarm

-- 
 .''`.  John Paul Adrian Glaubitz
: :' :  Debian Developer - glaubitz-8fiUuRrzOP0dnm+yROfE0A@public.gmane.org
`. `'   Freie Universitaet Berlin - glaubitz-1Olz3AKvcsuAKZTfuerNgRvVK+yQ3ZXh@public.gmane.org
  `-    GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913

Re: Testsuite failures on sparc64 due to unaligned access since 1.5.0

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 1310 bytes --]

On Fri, Dec 11, 2020 at 03:14:26PM +0100, John Paul Adrian Glaubitz wrote:
> Hi!
> 
> On 12/11/20 2:07 AM, David Gibson wrote:
> > Right, alignment issues have been a perennial problem.  We find some
> > platform that has alignment constraints that break, fix that, then
> > someone finds the alignment fixups cause horrible performance
> > problems, repeat ad nauseum.
> 
> From my discussions with GCC developers I have learnt that it shouldn't
> be a problem to use memcpy() in such cases to avoid aliasing problems
> resulting in unaligned access.
> 
> GCC will just optimize memcpy() in cases where an unaligned access is
> faster.
> 
> > There's been another round of this pretty recently, which is probably
> > the cause of the fixup.  I just hope the re-optimizations that Tom
> > Rini has in his pipeline don't break things again for you.
> 
> The GCC compile farm has two fast SPARC servers [1] running Debian unstable,
> so I suggest testing such changes there before merging them.

It's just restoring the historical behavior, and Rob was just saying
this was a change in dtc and not libfdt, so should be fine.  I sent out
v3 last night, but I don't see it in the archives yet, if I sent it to
you directly would you be able to give it a quick spin?  Thanks!

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: Testsuite failures on sparc64 due to unaligned access since 1.5.0

[John Paul Adrian Glaubitz]

Hi!

On 12/11/20 3:41 PM, Tom Rini wrote:
>> From my discussions with GCC developers I have learnt that it shouldn't
>> be a problem to use memcpy() in such cases to avoid aliasing problems
>> resulting in unaligned access.
>>
>> GCC will just optimize memcpy() in cases where an unaligned access is
>> faster.

I meant "optimize memcpy() out", sorry.

> It's just restoring the historical behavior, and Rob was just saying
> this was a change in dtc and not libfdt, so should be fine.  I sent out
> v3 last night, but I don't see it in the archives yet, if I sent it to
> you directly would you be able to give it a quick spin?  Thanks!

Sure, no problem. Will do.

Adrian

-- 
 .''`.  John Paul Adrian Glaubitz
: :' :  Debian Developer - glaubitz-8fiUuRrzOP0dnm+yROfE0A@public.gmane.org
`. `'   Freie Universitaet Berlin - glaubitz-1Olz3AKvcsuAKZTfuerNgRvVK+yQ3ZXh@public.gmane.org
  `-    GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913

Re: Testsuite failures on sparc64 due to unaligned access since 1.5.0

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 1494 bytes --]

On Fri, Dec 11, 2020 at 08:03:42AM -0600, Rob Herring wrote:
> On Thu, Dec 10, 2020 at 7:09 PM David Gibson
> <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org> wrote:
> >
> > On Thu, Dec 10, 2020 at 10:04:47AM +0100, John Paul Adrian Glaubitz wrote:
> > > On 12/10/20 10:01 AM, John Paul Adrian Glaubitz wrote:
> > > > I have not done any bisecting yet, but this is most likely a result of someone casting
> > > > a pointer of type A into a pointer of type B which is normally not allowed but unfortunately
> > > > also not something that gcc complains about.
> > >
> > > Hmm, it seems someone fixed the issue on git master already:
> >
> > Right, alignment issues have been a perennial problem.  We find some
> > platform that has alignment constraints that break, fix that, then
> > someone finds the alignment fixups cause horrible performance
> > problems, repeat ad nauseum.
> >
> > There's been another round of this pretty recently, which is probably
> > the cause of the fixup.  I just hope the re-optimizations that Tom
> > Rini has in his pipeline don't break things again for you.
> 
> I think the case here is dtc, not libfdt. My guess is something to do
> with marker handling writing out dts.

Ah, sorry, I wasn't paying sufficiently close attention.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]