On 12/16/2013, 11:44 AM, Jeff Law wrote:
On 12/11/13 09:49, Vladimir Makarov wrote:
I could prevent use validize_mem in rs6000.c but I prefer to do it
in change_addres_1 as other targets might have the same problem and it
is better to have one for all solution.
It's certainly been speculated that a fair amount of the "target
dependent" stuff done in LEGITIMIZE_ADDRESS and
RELOAD_LEGITIMIZE_ADDRESS could be factored out, parameterized and done
in a target independent way.
Still it does not fully solve the problem as insn
r257:DI=[unspec[`*.LC29',%2:DI] 45] cant be recognized as
*movdi... pattern has operand predicates rejecting memory because of
invalid address. To fix this a change in general_operand is done. As
LRA can not work properly with regular insn recognition, I added an
assert for this in lra_set_insn_recog_data to figure out this
situation earlier.
I'm getting a bit concerned about the number of places outside LRA that
test lra_in_progress. In theory, other code shouldn't want/need to care
about LRA vs reload. Both serve the same purpose, lra just does it
better :-)
Right now the numbers are pretty comparable, but let's try and avoid
sprinkling this stuff too far and wide.
I'd like to keep it under control too. For me it would be
uncomfortable to have more lra_in_progress occurrences than
reload_in_progress ones. The numbers (with taking this patch into
account) are very close but lra_in_progress places are still less than
reload_in_progress.
I can see how these might be needed from time to time
Again, LRA has a very good code for legitimize address by itself and
it is better to use it.
I have no trouble believing that.
LEGITIMIZE_ADDRESS/LEGITIMIZE_RELOAD_ADDRESS were really hacks to avoid
generating stupid code without having to do any serious surgery,
particularly in reload.
The patch was successfully bootstrapped and tested on i686, x86_64,
and PPC64.
Ok to commit?
2013-12-11 Vladimir Makarov <vmaka...@redhat.com>
PR rtl-optimization/59466
* emit-rtl.c (change_address_1): Don't validate address for LRA.
* recog.c (general_operand): Accept any memory for LRA.
* lra.c (lra_set_insn_recog_data): Add an assert.
Index: emit-rtl.c
===================================================================
--- emit-rtl.c (revision 205870)
+++ emit-rtl.c (working copy)
@@ -1951,7 +1951,9 @@ change_address_1 (rtx memref, enum machi
&& (!validate || memory_address_addr_space_p (mode, addr, as)))
return memref;
- if (validate)
+ /* Don't validate address for LRA. LRA can make the address valid
+ by itself in most efficient way. */
+ if (validate && !lra_in_progress)
Presumably if LRA can't find a reasonable way to make the address valid,
it falls back to copying the address into a register?
Yes, it can always fall back to putting address into a pseudo if it can
not generate a better code.
Thanks, Jeff.
