Re: [AArch64] Fix ICEs in aarch64_print_operand

[Richard Sandiford <richard dot sandiford at linaro dot org>]

Christophe Lyon <christophe.lyon@linaro.org> writes:
> Hi Richard,
> On 7 December 2017 at 10:31, James Greenhalgh <james.greenhalgh@arm.com> wrote>> On Tue, Dec 05, 2017 at 05:57:37PM +0000, Richard Sandiford wrote:
>>> Three related regression fixes:
>>>
>>> - We can't use asserts like:
>>>
>>>     gcc_assert (GET_MODE_SIZE (mode) == 16);
>>>
>>>   in aarch64_print_operand because it could trigger for invalid user input.
>>>
>>> - The output_operand_lossage in aarch64_print_address_internal:
>>>
>>>     output_operand_lossage ("invalid operand for '%%%c'", op);
>>>
>>>   wasn't right because "op" is an rtx_code enum rather than the
>>>   prefix character.
>>>
>>> - aarch64_print_operand_address shouldn't call output_operand_lossage
>>>   (because it doesn't have a prefix code) but instead fall back to
>>>   output_addr_const.
>>>
>>> Tested on aarch64-linux-gnu.  OK to install?
>>
>> OK.
>>
>> Thanks,
>> James
>>
>>>
>>> Thanks,
>>> Richard
>>>
>>>
>>> 2017-12-05  Richard Sandiford  <richard.sandiford@linaro.org>
>>>
>>> gcc/
>>>       * config/aarch64/aarch64.c (aarch64_print_address_internal): Return
>>>       a bool success value.  Don't call output_operand_lossage here.
>>>       (aarch64_print_ldpstp_address): Return a bool success value.
>>>       (aarch64_print_operand_address): Call output_addr_const if
>>>       aarch64_print_address_internal fails.
>>>       (aarch64_print_operand): Don't assert that the mode is 16 bytes for
>>>       'y'; call output_operand_lossage instead.  Call output_operand_lossage
>>>       if aarch64_print_ldpstp_address fails.
>>>
>>> gcc/testsuite/
>>>       * gcc.target/aarch64/asm-2.c: New test.
>>>       * gcc.target/aarch64/asm-3.c: Likewise.
>>>
>
> The new test gcc.target/aarch64/asm-2.c ICEs when compiled with -mabi=ilp32:
>
> /gcc/testsuite/gcc.target/aarch64/asm-2.c: In function 'f':
> /gcc/testsuite/gcc.target/aarch64/asm-2.c:10:1: internal compiler
> error: in aarch64_print_address_internal, at
> config/aarch64/aarch64.c:5636
> 0xf2afd3 aarch64_print_address_internal
>         /gcc/config/aarch64/aarch64.c:5636
> 0xf2affd aarch64_print_operand_address
>         /gcc/config/aarch64/aarch64.c:5733
> 0x7fdd43 output_address(machine_mode, rtx_def*)
>         /gcc/final.c:3913
> 0x801288 output_asm_insn(char const*, rtx_def**)
>         /gcc/final.c:3770
> 0x802437 final_scan_insn(rtx_insn*, _IO_FILE*, int, int, int*)
>         /gcc/final.c:2673
> 0x802a1a final(rtx_insn*, _IO_FILE*, int)
>         /gcc/final.c:2052
> 0x8035ab rest_of_handle_final
>         /gcc/final.c:4498
> 0x8035ab execute
>         /gcc/final.c:4572
>
> Can you check?

I think that's a separate preexisting problem.  Could you file a PR?

Personally I'd just remove the assert, but I'm guessing that wouldn't
be acceptable...

Thanks,
Richard