This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [patch,avr] PR78883: Implement CANNOT_CHANGE_MODE_CLASS.
2017-01-02 19:22 GMT+04:00 Georg-Johann Lay <avr@gjlay.de>:
> On 02.01.2017 15:54, Dominik Vogt wrote:
>>
>> On Mon, Jan 02, 2017 at 03:47:43PM +0100, Georg-Johann Lay wrote:
>>>
>>> This fixes PR78883 which is a problem in reload revealed by a
>>> change to combine.c. The fix is as proposed by Segher: implement
>>> CANNOT_CHANGE_MODE_CLASS.
>>>
>>> Ok for trunk?
>>>
>>> Johann
>>>
>>>
>>> gcc/
>>> PR target/78883
>>> * config/avr/avr.h (CANNOT_CHANGE_MODE_CLASS): New define.
>>> * config/avr/avr-protos.h (avr_cannot_change_mode_class): New
>>> proto.
>>> * config/avr/avr.c (avr_cannot_change_mode_class): New function.
>>>
>>> gcc/testsuite/
>>> PR target/78883
>>> * gcc.c-torture/compile/pr78883.c: New test.
>>
>>
>>> Index: config/avr/avr-protos.h
>>> ===================================================================
>>> --- config/avr/avr-protos.h (revision 244001)
>>> +++ config/avr/avr-protos.h (working copy)
>>> @@ -111,7 +111,7 @@ extern int _reg_unused_after (rtx_insn *
>>> extern int avr_jump_mode (rtx x, rtx_insn *insn);
>>> extern int test_hard_reg_class (enum reg_class rclass, rtx x);
>>> extern int jump_over_one_insn_p (rtx_insn *insn, rtx dest);
>>> -
>>> +extern int avr_cannot_change_mode_class (machine_mode, machine_mode,
>>> enum reg_class);
>>> extern int avr_hard_regno_mode_ok (int regno, machine_mode mode);
>>> extern void avr_final_prescan_insn (rtx_insn *insn, rtx *operand,
>>> int num_operands);
>>> Index: config/avr/avr.c
>>> ===================================================================
>>> --- config/avr/avr.c (revision 244001)
>>> +++ config/avr/avr.c (working copy)
>>> @@ -11833,6 +11833,21 @@ jump_over_one_insn_p (rtx_insn *insn, rt
>>> }
>>>
>>>
>>> +/* Worker function for `CANNOT_CHANGE_MODE_CLASS'. */
>>> +
>>> +int
>>> +avr_cannot_change_mode_class (machine_mode from, machine_mode to,
>>> + enum reg_class /* rclass */)
>>> +{
>>> + /* We cannot access a hard register in a wider mode, for example we
>>> + must not access (reg:QI 31) as (reg:HI 31). HARD_REGNO_MODE_OK
>>> + would avoid such hard regs, but reload would generate it anyway
>>> + from paradoxical subregs of mem, cf. PR78883. */
>>> +
>>> + return GET_MODE_SIZE (to) > GET_MODE_SIZE (from);
>>
>>
>> I understand how this fixes the ICE, but is it really necessary to
>> suppress conversions to a wider mode for lower numbered registers?
>
>
> If there is a better hook, I'll propose an according patch.
>
> My expectation was that HARD_REGNO_MODE_OK would be enough to keep
> reload from putting HI into odd registers (and in particular into R31).
> But this is obviously not the case...
>
> And internals are not very helpful here. It only mentions modifying
> ordinary subregs of pseudos, but not paradoxical subreg of memory.
>
> What's also astonishing me is that this problem never popped up
> during the last > 15 years of avr back-end.
May be it's a related problem: https://gcc.gnu.org/ml/gcc/2011-02/msg00444.html
>
> Johann
>
>
>
>>> +}
>>> +
>>> +
>>> /* Worker function for `HARD_REGNO_MODE_OK'. */
>>> /* Returns 1 if a value of mode MODE can be stored starting with hard
>>> register number REGNO. On the enhanced core, anything larger than
>>> Index: config/avr/avr.h
>>> ===================================================================
>>> --- config/avr/avr.h (revision 244001)
>>> +++ config/avr/avr.h (working copy)
>>> @@ -216,6 +216,9 @@ These two properties are reflected by bu
>>>
>>> #define MODES_TIEABLE_P(MODE1, MODE2) 1
>>>
>>> +#define CANNOT_CHANGE_MODE_CLASS(MFROM, MTO, RCLASS) \
>>> + avr_cannot_change_mode_class (MFROM, MTO, RCLASS)
>>> +
>>> enum reg_class {
>>> NO_REGS,
>>> R0_REG, /* r0 */
>>> Index: testsuite/gcc.c-torture/compile/pr78883.c
>>> ===================================================================
>>> --- testsuite/gcc.c-torture/compile/pr78883.c (nonexistent)
>>> +++ testsuite/gcc.c-torture/compile/pr78883.c (working copy)
>>> @@ -0,0 +1,12 @@
>>> +/* { dg-do compile } */
>>> +
>>> +int foo (int *p)
>>> +{
>>> + int i;
>>> + for (i = 0; i < 5; i++)
>>> + {
>>> + if (p[i] & 1)
>>> + return i;
>>> + }
>>> + return -1;
>>> +}
>>
>>
>> Ciao
>>
>> Dominik ^_^ ^_^
>>
>