Ping!
Regards
Senthil
Senthil Kumar Selvaraj writes:
> Hi,
>
> The below patch fixes what I think are a couple of problems with
> reload.c:find_valid_class_1.
>
> First, even if no regno is in_hard_reg_set_p, it goes ahead and
> considers rclass as valid. bad is set only if a regno is in the reg
> class *and* HARD_REGNO_MODE_OK is false - if both are false, bad isn't
> set and the reload gets a wrong rclass. If that happens to be the best
> one, this eventually leads to find_reg running out of registers to
> spill, because the chosen rclass won't have enough regs.
>
> Second, it expects every regno in rclass to be valid for mode i.e., if
> any regno fails HARD_REGNO_MODE_OK, it rejects the rclass. The
> comments in the original commit for find_valid_class_1 say atleast one
> regno is ok. This was updated to say "class which contains only
> registers" when in_hard_reg_set_p was introduced in place of just
> TEST_HARD_REG_BIT.
>
> Is it meant to really test all regs? For the avr target, all regs are
> 8 bits wide, and HARD_REGNO_MODE_OK returns false for odd regnos if
> mode != QImode. With the current code, it ignores a bunch of otherwise
> perfectly legal reg classes.
>
> To fix the first problem, the patch adds regno_in_rclass_mode to track
> whether there's atleast one regno in hard_reg_set_p. To fix the second
> problem, the patch sets bad initially, and resets it if
> HARD_REGNO_MODE_OK succeeded for a regno, thus breaking the loop.
>
> Of course, if both my points above are valid, we can do away with
> regno_in_rclass_mode, just bad would do.
>
> Does this make sense? I ran a reg test for the avr target with a
> slightly older version of this patch, it did not show any regressions.
> If this is the right fix, I'll make sure to run reg tests on x86_64
> after backporting to a gcc version where that target used reload.
>
> Regards
> Senthil
>
>
> Index: reload.c
> ===================================================================
> --- reload.c (revision 240185)
> +++ reload.c (working copy)
> @@ -714,17 +714,22 @@
>
> for (rclass = 1; rclass < N_REG_CLASSES; rclass++)
> {
> - int bad = 0;
> - for (regno = 0; regno < FIRST_PSEUDO_REGISTER && !bad; regno++)
> - {
> - if (in_hard_reg_set_p (reg_class_contents[rclass], mode, regno)
> - && !HARD_REGNO_MODE_OK (regno, mode))
> - bad = 1;
> - }
> -
> - if (bad)
> - continue;
> + int bad = 1;
> + int regno_in_rclass_mode = 0;
>
> + for (regno = 0; regno < FIRST_PSEUDO_REGISTER && bad; regno++)
> + {
> + if (in_hard_reg_set_p (reg_class_contents[rclass], mode, regno))
> + {
> + regno_in_rclass_mode = 1;
> + if (HARD_REGNO_MODE_OK (regno, mode))
> + bad = 0;
> + }
> + }
> +
> + if (bad || !regno_in_rclass_mode)
> + continue;
> +
> cost = register_move_cost (outer, (enum reg_class) rclass, dest_class);
>
> if ((reg_class_size[rclass] > best_size
