This is the mail archive of the
mailing list for the GCC project.
Re: [PATCH]: PR 11271 Handle reloading of R+R+CONST for any reg
- From: Eric Botcazou <ebotcazou at libertysurf dot fr>
- To: Ulrich Weigand <weigand at i1 dot informatik dot uni-erlangen dot de>
- Cc: rearnsha at arm dot com (Richard Earnshaw),gcc-patches at gcc dot gnu dot org
- Date: Tue, 18 Nov 2003 19:46:13 +0100
- Subject: Re: [PATCH]: PR 11271 Handle reloading of R+R+CONST for any reg
- References: <200311142318.AAA10302@faui1d.informatik.uni-erlangen.de>
> This test can not only have false positives, but also false negatives.
> In fact, I do not understand the reason for that test at all -- the
> documented meaning of LEGITIMATE_CONSTANT_P says nothing about this.
Yes, I think LEGITIMATE_CONSTANT_P is abused here.
> If I understand that thread correctly, the issue there was whether or
> not the target of the reload was the same as the REG inside the PLUS
> or not. This issue could be fixed -apart from in
> find_reloads_address_part- by changing either push_reload or gen_reload.
Yes, but the root problem is IMHO really that find_reloads_address_part
should push 2 reloads instead of only 1 in this case. I think that any other
fix (including my patch) is only papering over the problem.
> The situation on s390 is quite different. In the first place, all that
> code that tries to reload PLUSes in gen_reload is fundamentally broken
> for s390 because it generates RTL for a regular (arithmetic) addition
> pattern, while we need a load-address pattern -- this is really different
> on s390 because LOAD ADDRESS only performs a *31-bit* addition.
Thanks for clarifying this. The situation is indeed more intricate than on
> What gen_reload tries to generate will not match any insn pattern on
> s390, because we do not have any instruction to perform a 32-bit
> addition *without clobbering the condition code*. On the other hand,
> If gen_reload does finally use gen_add2_insn it will emit an insn
> that does clobber the (possibly live at this point!) condition code.
> [ Actually, just about every use of gen_add2_insn in reload1.c is
> fundamentally broken for that very same reason. ]
I recently came accross the very same problem with gen_add2_insn in
postreload.c on i386. And your remark is a bit frightening, because it
appears that every single use of gen_add2_insn might be unsafe...
> IMO the in the PLUS case in find_reloads_address_part the test for
> LEGITIMATE_CONSTANT_P / PREFERRED_RELOAD_CLASS is completely bogus.
> Instead, I think that a PLUS to be reloaded should be treated as
> an address (which it always is, actually!), and be fixed up via
> find_reloads_address if necessary, possibly like so:
> static void
> find_reloads_address_part (rtx x, rtx *loc, enum reg_class class,
> enum machine_mode mode, int opnum,
> enum reload_type type, int ind_levels)
> if (CONSTANT_P (x)
> && (! LEGITIMATE_CONSTANT_P (x)
> || PREFERRED_RELOAD_CLASS (x, class) == NO_REGS))
> rtx tem;
> tem = x = force_const_mem (mode, x);
> find_reloads_address (mode, &tem, XEXP (tem, 0), &XEXP (tem, 0),
> opnum, type, ind_levels, 0);
> else if (GET_CODE (x) == PLUS)
> find_reloads_address (VOIDmode, (rtx *)0, x, loc,
> opnum, type, ind_levels, 0);
> push_reload (x, NULL_RTX, loc, (rtx*) 0, class,
> mode, VOIDmode, 0, 0, opnum, type);
I think this would cure the SH4 bug too.