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