This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: bswap PRs 69714, 67781
- From: John David Anglin <dave dot anglin at bell dot net>
- To: Bernd Schmidt <bschmidt at redhat dot com>
- Cc: GCC Patches <gcc-patches at gcc dot gnu dot org>, "Thomas Preud'homme" <thomas dot preudhomme at foss dot arm dot com>
- Date: Fri, 12 Feb 2016 21:36:30 -0500
- Subject: Re: bswap PRs 69714, 67781
- Authentication-results: sourceware.org; auth=none
- References: <56BE083B dot 3090806 at redhat dot com>
On 2016-02-12, at 11:28 AM, Bernd Schmidt wrote:
> I'm attaching a full patch. John David Anglin is currently running tests on PA which may take a while. He's confirmed it fixes his testcase, and the earlier 67781 testcase seems to be cured as well (only looked at generated assembly, don't have a full cross environment - Thomas, can you verify?)
> I'm also doing an x86_64 test run. Ok everywhere if it passes? I'm also attaching a version of the testcase (LGPL, from ffmpeg) which I'd also apply to gcc.dg/torture if David (or Thomas) can confirm it works on a big-endian target after the fix.
I have tested the patch on hppa2.0w-hp-hpux11.11 and hppa-unknown-linux-gnu so far. It fixes the original testcase
and I didn't see any regressions related to this change. I also have a private communication that it also fixes the mips
testcase from Debian bug #813858.
As far as the avcrc.c reduced testcase, it didn't trigger the original bug on hppa-unknown-linux-gnu.
>
>
> There is room for more improvement in this code - hopefully we can get to that in gcc-7. As mentioned, it does not use the bytepos in the final replacement. We could extend the code to recognize smaller sub-bswaps at an offset. Currently, only one of the two rotates involved in a 32-bit bswap is recognized.
>
> Also, for the crc testcase, the transformation actually pessimizes the code by inserting an extra memory load. If we already have loaded a larger value, we should build the subpart using a shift and convert.
I noticed that. I'm testing a patch to add bswap patterns to pa.md. The PA 2.0 architecture manual actually
gives some optimized sequences for byte swap operations with half-word, word and double-word operands.
The compiler might be able to duplicate the half-word and word sequences, but there is no way it can
duplicate the double-word sequence. It uses half-word instructions that aren't exposed in PA backend.
Dave
--
John David Anglin dave.anglin@bell.net