[patch,avr] PR78883: Implement CANNOT_CHANGE_MODE_CLASS.

[Georg-Johann Lay]

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.

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 ^_^  ^_^
>