This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH][AArch64] Increase static buffer size in aarch64_rewrite_selected_cpu
- From: Kyrill Tkachov <kyrylo dot tkachov at arm dot com>
- To: James Greenhalgh <james dot greenhalgh at arm dot com>
- Cc: GCC Patches <gcc-patches at gcc dot gnu dot org>, Marcus Shawcroft <Marcus dot Shawcroft at arm dot com>, Richard Earnshaw <Richard dot Earnshaw at arm dot com>
- Date: Tue, 21 Apr 2015 09:19:09 +0100
- Subject: Re: [PATCH][AArch64] Increase static buffer size in aarch64_rewrite_selected_cpu
- Authentication-results: sourceware.org; auth=none
- References: <55352847 dot 8080302 at arm dot com> <20150420203001 dot GB7414 at arm dot com>
On 20/04/15 21:30, James Greenhalgh wrote:
On Mon, Apr 20, 2015 at 05:24:39PM +0100, Kyrill Tkachov wrote:
Hi all,
When trying to compile a testcase with -mcpu=cortex-a57+crypto+nocrc I got
the weird assembler error:
Assembler messages:
Error: missing architectural extension
Error: unrecognized option -mcpu=cortex-a57+crypto+no
The problem is the aarch64_rewrite_selected_cpu that is used to rewrite -mcpu
for big.LITTLE options has a limit of 20 characters in what it handles, which
we can exhaust quickly if we specify architectural extensions in a
fine-grained manner.
This patch increases that character limit to 128 and adds an assert to
confirm that no bad things happen.
You've implemented this as a hard ICE, was that intended?
Yeah, the idea is that before this we would silently truncate i.e. do the wrong thing.
Now, if we exceed the limit we ICE. I don't think it should be a user error because
it's not really the user's fault that the compiler doesn't handle crazy long strings
but handling arbitrary large strings would make this function more complex than I think
is needed for the majority of cases. If you plan to rewrite this in the future, we can
revisit that.
It also fixes another problem: If we pass a big.LITTLE combination with
feature modifiers like: -mcpu=cortex-a57.cortex-a53+nosimd
the code will truncate everything after '.', thus destroying the extensions
that we want to pass. The patch adds code to stitch the extensions back on
after the LITTLE cpu is removed.
UGH, I should not be allowed near strings! This code is on my list of
things I'd love to rewrite to this year! For now, this is OK and please
also queue it for 5.2 when that opens for patches.
Committed to trunk with r222258.
Thanks for looking at it,
Kyrill
Ok for trunk?
Yes, thanks. And sorry again for introducing this in the first place.
James