This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: PATCH [10/n]: Prepare x32: PR rtl-optimization/49114: Reload failed to handle (set reg:X (plus:X (subreg:X (reg:Y) 0) (const_int


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 ...

Bye,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]