This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH][AArch64] Fix ICE caused in aarch64_simd_valid_immediate
- From: "Richard Earnshaw (lists)" <Richard dot Earnshaw at arm dot com>
- To: Sudi Das <Sudi dot Das at arm dot com>, Jakub Jelinek <jakub at redhat dot com>
- Cc: "gcc-patches at gcc dot gnu dot org" <gcc-patches at gcc dot gnu dot org>, nd <nd at arm dot com>, "sellcey at cavium dot com" <sellcey at cavium dot com>, Marcus Shawcroft <Marcus dot Shawcroft at arm dot com>, James Greenhalgh <James dot Greenhalgh at arm dot com>
- Date: Fri, 6 Oct 2017 14:01:21 +0100
- Subject: Re: [PATCH][AArch64] Fix ICE caused in aarch64_simd_valid_immediate
- Authentication-results: sourceware.org; auth=none
- References: <DB5PR08MB10482E3B8EA8E199729AA2CE98710@DB5PR08MB1048.eurprd08.prod.outlook.com> <20171006101121.GM18588@tucnak> <DB5PR08MB1048D713B23B6C33EF242A5198710@DB5PR08MB1048.eurprd08.prod.outlook.com>
On 06/10/17 12:01, Sudi Das wrote:
>
> Hi Jakub
>
> I have modified the entries:
>
> *** gcc/ChangeLog ***
>
> 2017-10-05 Sudakshina Das <sudi.das@arm.com>
>
> PR target/82440
> * config/aarch64/predicates.md (aarch64_reg_or_orr_imm): Changed to
> to only call aarch64_simd_valid_immediate on CONST_VECTORs.
You don't need to say 'Changed to' (or even 'Changed to to' :-). Simply
say 'Only call ...'.
> (aarch64_reg_or_bic_imm): Likewise.
>
> *** gcc/testsuite/ChangeLog ***
>
> 2017-10-05 Sudakshina Das <sudi.das@arm.com>
>
> * gcc.target/aarch64/bic_imm_1.c: New test.
> * gcc.target/aarch64/orr_imm_1.c: Likewise..
too many full stops.
OK with those nits fixed.
R.
>
>
> Thanks
> Sudi
>
>
> From: Jakub Jelinek <jakub@redhat.com>
> Sent: Friday, October 6, 2017 11:11 AM
> To: Sudi Das
> Cc: gcc-patches@gcc.gnu.org; nd; sellcey@cavium.com; Marcus Shawcroft; Richard Earnshaw; James Greenhalgh
> Subject: Re: [PATCH][AArch64] Fix ICE caused in aarch64_simd_valid_immediate
>
> On Fri, Oct 06, 2017 at 09:52:35AM +0000, Sudi Das wrote:
>> This patch is a fix for PR 82440.
>> The predicates aarch64_reg_or_bic_imm and aarch64_reg_or_orr_imm missed out on
>> checking for a CONST_VECTOR before calling aarch64_simd_valid_immediate function.
>> Also I think James forgot to add the test cases in the original patch submitted.
>>
>> Testing done : Checked for regressions on bootstrapped aarch64-none-linux-gnu.
>> Ok for trunk?
>>
>> Thanks
>> Sudi
>>
>> The ChangeLog entry is as follows:
>>
>> *** gcc/ChangeLog ***
>>
>> 2017-10-06 Sudakshina Das <sudi.das@arm.com>
>>
>> PR target/82440
>> * config/aarch64/predicates.md (aarch64_reg_or_orr_imm): Modified.
>> (aarch64_reg_or_bic_imm): Likewise.
>
> I'll defer the actual review to aarch64 maintainers, just want to say that
> this is not a correct ChangeLog entry. You should say what has changed, not
> just that something has changed. Something like
> Only call aarch64_simd_valid_immediate on CONST_VECTORs.
> or similar.
>
> Jakub
>
>