> Am 16.10.2018 um 16:20 schrieb Jeff Law <l...@redhat.com>:
>
> On 10/16/18 3:17 AM, Ilya Leoshkevich wrote:
>> Bootstrap and regtest are running on x86_64-redhat-linux.
>>
>> Insns in FROM..TO range might not be recognized yet, and their
>> corresponding entries in lra_insn_recog_data[] might be NULLs. In the
>> code from PR87596 the problematic insn is merely
>>
>> (note 148 154 68 7 NOTE_INSN_DELETED)
>>
>> however I assume that non-note insns may occur as well. So make sure
>> they are recognized before touching their lra_insn_recog_data_t.
>>
>> gcc/ChangeLog:
>>
>> 2018-10-16 Ilya Leoshkevich <i...@linux.ibm.com>
>>
>> PR rtl-optimization/87596
>> * lra-constraints.c (spill_hard_reg_in_range): Use
>> lra_get_insn_recog_data () instead of lra_insn_recog_data[]
>> for instructions in FROM..TO range.
>>
>> gcc/testsuite/ChangeLog:
>>
>> 2018-10-16 Ilya Leoshkevich <i...@linux.ibm.com>
>>
>> PR rtl-optimization/87596
>> * gcc.target/i386/pr87596.c: New test.
> I think this is just papering over the problem.
>
> AFAICT for something like a deleted note lra_get_insn_recog_data is
> going to call lra_set_insn_recog_data. Given that INSN will be a NOTE I
> think icode will end up being -1 and we'll eventually get into this clause:
>
> if (icode < 0)
> {
> int nop, nalt;
> machine_mode operand_mode[MAX_RECOG_OPERANDS];
> const char *constraints[MAX_RECOG_OPERANDS];
>
> nop = asm_noperands (PATTERN (insn));
>
> And I don't think we should be trying to extract the PATTERN of a note.
>
> I think you need to either filter out the non-insn thingies in
> spill_hard_reg_in_range, or go with your patch plus beefing up the
> checks in lra_set_insn_recog_data.
>
> jeff
>
Thanks, you’re absolutely right!
I re-ran the test with --enable-checking=all, and it crashed in another
branch of that if statement:
else
{
insn_extract (insn); /* Also uses PATTERN (). */
data->insn_static_data = insn_static_data
I looked at the usage patterns of lra_insn_recog_data[] and
lra_get_insn_recog_data (), and noticed that we use the former directly
only when the insn in question is taken from insn_bitmap, and the latter
in all other cases. Furthermore, lra_get_insn_recog_data () is normally
guarded by INSN_P () or something similar.
So I plan to keep using lra_get_insn_recog_data (), but with an extra
INSN_P () check.
