On 30/04/12 15:07, Richard Sandiford wrote:
> Richard Earnshaw <rearn...@arm.com> writes:
>> On 26/04/12 14:20, Jim MacArthur wrote:
>>> The current code in reg_fits_class_p appears to be incorrect; since
>>> offset may be negative, it's necessary to check both ends of the range
>>> otherwise an array overrun or underrun may occur when calling
>>> in_hard_reg_set_p. in_hard_reg_set_p should also be checked for each
>>> register in the range of regno .. regno+offset.
>>>
>>> A negative offset can occur on a big-endian target. For example, on
>>> AArch64, subreg_regno_offset (0, DImode, 0, TImode) returns -1.
>>>
>>> We discovered this problem while developing unrelated code for
>>> big-endian support in the AArch64 back end.
>>>
>>> I've tested this with an x86 bootstrap which shows no errors, and with
>>> our own AArch64 back end.
>>>
>>> Ok for trunk?
>>>
>>> gcc/Changelog entry:
>>>
>>> 2012-04-26 Jim MacArthur<jim.macart...@arm.com>
>>> * recog.c (reg_fits_class_p): Check every register between regno and
>>> regno+offset is in the hard register set.
>>>
>>
>> OK.
>>
>> R.
>>
>>>
>>> reg-fits-class-9
>>>
>>>
>>> diff --git a/gcc/recog.c b/gcc/recog.c
>>> index 8fb96a0..825bfb1 100644
>>> --- a/gcc/recog.c
>>> +++ b/gcc/recog.c
>>> @@ -2759,14 +2759,28 @@ bool
>>> reg_fits_class_p (const_rtx operand, reg_class_t cl, int offset,
>>> enum machine_mode mode)
>>> {
>>> - int regno = REGNO (operand);
>>> + unsigned int regno = REGNO (operand);
>>>
>>> if (cl == NO_REGS)
>>> return false;
>>>
>>> - return (HARD_REGISTER_NUM_P (regno)
>>> - && in_hard_reg_set_p (reg_class_contents[(int) cl],
>>> - mode, regno + offset));
>>> + /* We should not assume all registers in the range regno to regno +
>>> offset are
>>> + valid just because each end of the range is. */
>>> + if (HARD_REGISTER_NUM_P (regno) && HARD_REGISTER_NUM_P (regno + offset))
>>> + {
>>> + unsigned int i;
>>> +
>>> + unsigned int start = MIN (regno, regno + offset);
>>> + unsigned int end = MAX (regno, regno + offset);
>>> + for (i = start; i <= end; i++)
>>> + {
>>> + if (!in_hard_reg_set_p (reg_class_contents[(int) cl],
>>> + mode, i))
>>> + return false;
>>> + }
>
> This doesn't look right to me. We should still only need to check
> in_hard_reg_set_p for one register number. I'd have expected
> something like:
>
> return (HARD_REGISTER_NUM_P (regno)
> && HARD_REGISTER_NUM_P (regno + offset)
> && in_hard_reg_set_p (reg_class_contents[(int) cl],
> mode, regno + offset));
>
> instead.
>
> Richard
>
Hmm, looking at the comment that precedes the function makes me think
the actual implementation should be:
{
int regno = REGNO (operand) + offset;
...
return (HARD_REGISTER_NUM_P (regno)
&& HARD_REGISTER_NUM_P (end_hard_regno (regno, mode))
&& in_hard_reg_set_p (...., regno);
}
That is, the original regno isn't really interesting and what really
counts is the range regno + offset ... regno + offset +
NUM_HARD_REGS(mode) - 1
R.
