Jeff Law <l...@redhat.com> writes:
> On 6/6/19 6:33 AM, Richard Sandiford wrote:
>> Kugan Vivekanandarajah <kugan.vivekanandara...@linaro.org> writes:
>>> Hi Richard,
>>>
>>> On Thu, 6 Jun 2019 at 22:07, Richard Sandiford
>>> <richard.sandif...@arm.com> wrote:
>>>>
>>>> Kugan Vivekanandarajah <kugan.vivekanandara...@linaro.org> writes:
>>>>> Hi Richard,
>>>>>
>>>>> On Thu, 6 Jun 2019 at 19:35, Richard Sandiford
>>>>> <richard.sandif...@arm.com> wrote:
>>>>>>
>>>>>> Kugan Vivekanandarajah <kugan.vivekanandara...@linaro.org> writes:
>>>>>>> Hi Richard,
>>>>>>>
>>>>>>> Thanks for the review. Attached is the latest patch.
>>>>>>>
>>>>>>> For testcase like cond_arith_1.c, with the patch, gcc ICE in fwprop. I
>>>>>>> am limiting fwprop in cases like this. Is there a better fix for this?
>>>>>>> index cf2c9de..2c99285 100644
>>>>>>> --- a/gcc/fwprop.c
>>>>>>> +++ b/gcc/fwprop.c
>>>>>>> @@ -1358,6 +1358,15 @@ forward_propagate_and_simplify (df_ref use,
>>>>>>> rtx_insn *def_insn, rtx def_set)
>>>>>>> else
>>>>>>> mode = GET_MODE (*loc);
>>>>>>>
>>>>>>> + /* TODO. We can't get the mode for
>>>>>>> + (set (reg:VNx16BI 109)
>>>>>>> + (unspec:VNx16BI [
>>>>>>> + (reg:SI 131)
>>>>>>> + (reg:SI 106)
>>>>>>> + ] UNSPEC_WHILE_LO))
>>>>>>> + Thus, bailout when it is UNSPEC and MODEs are not compatible. */
>>>>>>> + if (GET_MODE_CLASS (mode) != GET_MODE_CLASS (GET_MODE (reg)))
>>>>>>> + return false;
>>>>>>> new_rtx = propagate_rtx (*loc, mode, reg, src,
>>>>>>> optimize_bb_for_speed_p (BLOCK_FOR_INSN (use_insn)));
>>>>>>
>>>>>> What specifically goes wrong? The unspec above isn't that unusual --
>>>>>> many unspecs have different modes from their inputs.
>>>>>
>>>>> cond_arith_1.c:38:1: internal compiler error: in paradoxical_subreg_p,
>>>>> at rtl.h:3130
>>>>> 0x135f1d3 paradoxical_subreg_p(machine_mode, machine_mode)
>>>>> ../../88838/gcc/rtl.h:3130
>>>>> 0x135f1d3 propagate_rtx
>>>>> ../../88838/gcc/fwprop.c:683
>>>>> 0x135f4a3 forward_propagate_and_simplify
>>>>> ../../88838/gcc/fwprop.c:1371
>>>>> 0x135f4a3 forward_propagate_into
>>>>> ../../88838/gcc/fwprop.c:1430
>>>>> 0x135fdcb fwprop
>>>>> ../../88838/gcc/fwprop.c:1519
>>>>> 0x135fdcb execute
>>>>> ../../88838/gcc/fwprop.c:1550
>>>>> Please submit a full bug report,
>>>>> with preprocessed source if appropriate.
>>>>>
>>>>>
>>>>> in forward_propagate_and_simplify
>>>>>
>>>>> use_set:
>>>>> (set (reg:VNx16BI 96 [ loop_mask_52 ])
>>>>> (unspec:VNx16BI [
>>>>> (reg:SI 92 [ _3 ])
>>>>> (reg:SI 95 [ niters.36 ])
>>>>> ] UNSPEC_WHILE_LO))
>>>>>
>>>>> reg:
>>>>> (reg:SI 92 [ _3 ])
>>>>>
>>>>> *loc:
>>>>> (unspec:VNx16BI [
>>>>> (reg:SI 92 [ _3 ])
>>>>> (reg:SI 95 [ niters.36 ])
>>>>> ] UNSPEC_WHILE_LO)
>>>>>
>>>>> src:
>>>>> (subreg:SI (reg:DI 136 [ ivtmp_101 ]) 0)
>>>>>
>>>>> use_insn:
>>>>> (insn 87 86 88 4 (parallel [
>>>>> (set (reg:VNx16BI 96 [ loop_mask_52 ])
>>>>> (unspec:VNx16BI [
>>>>> (reg:SI 92 [ _3 ])
>>>>> (reg:SI 95 [ niters.36 ])
>>>>> ] UNSPEC_WHILE_LO))
>>>>> (clobber (reg:CC 66 cc))
>>>>> ]) 4255 {while_ultsivnx16bi}
>>>>> (expr_list:REG_UNUSED (reg:CC 66 cc)
>>>>> (nil)))
>>>>>
>>>>> I think we calculate the mode to be VNx16BI which is wrong?
>>>>> because of which in propgate_rtx, !paradoxical_subreg_p (mode,
>>>>> GET_MODE (SUBREG_REG (new_rtx))))) ICE
>>>>
>>>> Looks like something I hit on the ACLE branch, but didn't have a
>>>> non-ACLE reproducer for (see 065881acf0de35ff7818c1fc92769e1c106e1028).
>>>>
>>>> Does the attached work? The current call is wrong because "mode"
>>>> is the mode of "x", not the mode of "new_rtx".
>>>
>>> Yes, attached patch works for this testcase. Are you planning to
>>> commit it to trunk. I will wait for this.
>>
>> Needs approval first. :-)
>>
>> The patch was originally bootstrapped & regtested on aarch64-linux-gnu,
>> but I'll repeat that for trunk and test x86_64-linux-gnu too.
> Assuming that passes this is fine.
> jeff
Thanks, applied as r272032.
Richard
