On Mon, Jun 27, 2011 at 7:47 AM, Ulrich Weigand <uweig...@de.ibm.com> wrote:
> H.J. Lu wrote:
>
>> When reload gets:
>>
>> (insn 588 587 589 28 (set (mem:DF (zero_extend:DI (plus:SI (subreg:SI
>> (reg/v/f:DI 182 [ b ]) 0)
>> (const_int 8 [0x8]))) [4 MEM[base: b_96(D), index:
>> D.15020_278, step: 8, offset: 0B]+0 S8 A64])
>> (reg:DF 340 [ D.14980 ])) spooles.c:291 106
>> {*movdf_internal_rex64}
>> (expr_list:REG_DEAD (reg:DF 340 [ D.14980 ])
>> (nil)))
>
> Reloading of PLUS expressions is a long-standing problem. gen_reload
> supports those only for PLUSes that look more or less like address
> computations, and then only the "usual" cases.
>
> Is the address above (once the pseudo reg:DI 182 is replaced by
> a hard reg) really a legitimate address on your platform? If not,
> this would need to be fixed at some earlier place.
>
> If this *is* a valid address (and just not valid for this particular
> insn pattern), the back-end needs to provide some means to reload to
> allow reloading of such expressions. This can be done either by
> providing an insn (plus post-reload splitter if necessary), or else
> defining a secondary reload to handle the case where additional
> registers are required. Assuming the generic gen_reload code is
> powerful enough to handle complex expressions like this is probably
> not wise ...
>
> In any case, however, gen_reload should not generate *wrong*
> code, so there's indeed a bug here.
>
> However, this:
>
>> - if (CONSTANT_P (op1) || MEM_P (op1) || GET_CODE (op1) == SUBREG
>> + if ((GET_CODE (op0) != SUBREG
>> + && (CONSTANT_P (op1) || MEM_P (op1)))
>> + || GET_CODE (op1) == SUBREG
>> || (REG_P (op1)
>> && REGNO (op1) >= FIRST_PSEUDO_REGISTER)
>> || (code != CODE_FOR_nothing
>
> doesn't look like the proper fix for all cases. The actual problem
> here is that this part of gen_reload takes the approach to transform
>
> out <- op0 + op1
>
> into
>
> out <- op0
> out <- out + op1
>
> which is invalid if writing to out clobbers op1.
>
> This means that:
>
> - The "if" testing whether to swap op0 and op1 should verify
> !reg_overlap_mentioned_p (out, op0)
>
> - Regardless of whether we swapped or not, there needs to be a
> gcc_assert (!reg_overlap_mentioned_p (out, op1));
> before the gen_reload (out, op0, opnum, type) line.
>
> There may still be cases where the algorithm of gen_reload doesn't
> work, but at least we'll get an ICE instead of wrong code now.
> Those cases will have to be fixed by the back-end as described
> above ...
>
The problem is reload 0 puts OP1 in OUT. Adding
gcc_assert (!reg_overlap_mentioned_p (out, op1));
doesn't help in reload 2. How can I check if OP1 overlaps with
OUT in previous reload?
--
H.J.
