On Tue, Nov 24, 2020 at 06:19:41AM +0000, Maciej W. Rozycki wrote: 

>  NB I find the reindentation resulting in `push_reload' awful, just as I 
> do either version of the massive logical expression involved.  Perhaps we 
> could factor these out into `static inline' functions sometime, and then 
> have them split into individual returns within?

I'm generally hesitant to do a lot of changes to the reload code base
at this stage.  The strategy rather is to move all remaining targets
over to LRA and then simply delete reload :-)

Given that you're modernizing the vax target, I'm wondering if you
shouldn't rather go all the way and move it over to LRA as well,
then you wouldn't be affected by any remaining reload deficiencies.
(The major hurdle so far was that LRA doesn't support CC0, but it
looks like you're removing that anyway ...)

> +      && (strict_low
> +       || (subreg_lowpart_p (in)
> +           && (CONSTANT_P (SUBREG_REG (in))
> +               || GET_CODE (SUBREG_REG (in)) == PLUS
> +               || strict_low

If we do this, then that "|| strict_low" clause is now redundant. 

> +           && REGNO (SUBREG_REG (in)) >= FIRST_PSEUDO_REGISTER
> +           && reg_equiv_mem (REGNO (SUBREG_REG (in)))
> +           && (mode_dependent_address_p
> +               (XEXP (reg_equiv_mem (REGNO (SUBREG_REG (in))), 0),
> +                MEM_ADDR_SPACE (reg_equiv_mem (REGNO (SUBREG_REG (in)))))))))

I guess this is not incorrect, but I'm wondering if it would be
*complete* -- there are other cases where reload replaces a pseudo
with a MEM even where reg_equiv_mem is null.

That said, if it fixes the test suite errors you're seeing, it would
probably be OK to go with just this minimal change -- unless we can
just move to LRA as mentioned above.

Bye,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU/Linux compilers and toolchain
  ulrich.weig...@de.ibm.com

Reply via email to