Reload and reg_equiv_constant
Richard Sandiford
rsandifo@redhat.com
Mon Aug 25 02:16:00 GMT 2003
Sorry in advance for the long message. And I'm sure this sort of thing
has been discussed before. But anyway...
Reload seems a bit too eager to force constants into memory, at least on
some architectures. And while looking into that, I wondered whether it
was doing enough to cater for SECONDARY_MEMORY_NEEDED.
Say we have:
(set (reg:SI r1) (reg:SI r2))
and:
r1 & r2 are pseudos,
r1 is allocated a general register,
r2 isn't, but is found to be equivalent to a constant C,
C isn't accepted by the movsi constraints
If there are two alternatives:
reg <- reg
reg <- mem
then both are treated as needing one reload (cost: 6 units).
However, this code in find_reloads:
/* We prefer to reload pseudos over reloading other things,
since such reloads may be able to be eliminated later.
If we are reloading a SCRATCH, we won't be generating any
insns, just using a register, so it is also preferred.
So bump REJECT in other cases. Don't do this in the
case where we are forcing a constant into memory and
it will then win since we don't want to have a different
alternative match then. */
if (! (GET_CODE (operand) == REG
&& REGNO (operand) >= FIRST_PSEUDO_REGISTER)
&& GET_CODE (operand) != SCRATCH
&& ! (const_to_mem && constmemok))
reject += 2;
will increase the cost of the reg <- reg alternative by 2 units,
deliberately making the reg <- mem one cheaper.
I can see that that makes sense for targets where symbol_refs are valid
addresses. But when they aren't, we'll end up having to reload the
constant pool address into a register. For example, on mips-linux-gnu,
we'd get the equivalent of:
.LC0: .word C
la temp,.LC0
lw r1,(temp)
See the attached stress.c for a brute-force example.
So, my question is: do we really need to prefer reg <- mem over
reg <- reg on targets that will need to reload the pool address?
Choosing reg <- reg will mean that the input reload will "rematerialise"
the constant (by calling the move expanders or by using the reload_in
pattern). And reload's optimisers are often able to remove any
redundant reg <- reg moves that this might create.
I tried two ways of making the reg <- mem alternative more costly:
- Disable the const_to_mem get-out clause above if symbol_refs aren't
valid addresses. [patch-1.diff]
- (More aggressive:) Count the expected symbol_ref reload as a full
extra reload. [patch-2.diff]
The patches are supposed to be mutually exclusive. They follow the
existing convention of testing for addressing modes in init_reload.
To see how the changes affect ports other than mips, I tried taking one
configuration from each backend[*] and diffing the output of c-torture.
I'd copied stress.c to c-torture first, so it's included in the analysis
below.
([*] aside from n32k & iq2000, which didn't build)
All files were compiled with: <dir>/xgcc -B<dir>/ -O2 -S foo.c -o foo.s
The results were as follows (ignore the last column for now ;):
CVS Patch 1 Patch 2
vs. vs. vs.
Patch 1 Patch 2 Patch 3
alpha-linux-gnu same better?
arc-elf n/a n/a
arm-elf same n/a
avr-elf n/a n/a
c4x-elf n/a n/a
cris-elf n/a n/a
d30v-elf n/a n/a
dsp16xx-elf same n/a
fr30-elf same n/a
frv-elf same n/a
h8300-elf n/a n/a
hppa-linux-gnu same better?
i370-linux-gnu same n/a
i686-linux-gnu n/a better?
ia64-linux-gnu same n/a
ip2k-elf same n/a
m32r-elf same n/a
m68hc11-elf n/a n/a
m68k-elf n/a n/a
mcore-elf ---- n/a
mips-linux-gnu better n/a
mmix n/a n/a
mn10300-elf n/a n/a
pdp11-elf n/a n/a
powerpc-linux-gnu better? same
s390-linux-gnu ---- ---- same
sh64-elf same better? n/a
sparc-linux-gnu better? worse better? (= patch 1)
v850-elf better? n/a
xstormy16-elf same n/a
vax-ultrix n/a n/a
xtensa-elf same n/a
Key to "CVS vs. Patch 1":
n/a: The target allows symbolic addresses, so the patch has no effect.
same: The target doesn't allow symbolic addresses but the patch
has no effect anyway. Presumably the md patterns had matching
constant constraints.
better: The output looks better to me. I'm not real judge of
powerpc, sparc or v850 code though, so I've attached the diffs
below.
There were two problems:
- mcore-elf ICEd for a few more tests. It seems that the movsi
patterns can't handle all legitimate constants when called
during reload.
- There are lots of s390-linux-gnu changesm, some of which
look better and some of which look worse. I've attached
the diff for reference, but...
As far as s390 goes, the init_reload test wasn't really supposed to
fail in the first place. The port _does_ treat constant pool addresses
as legitimate.
The problem is that the test in init_reload is for a general symbol_ref,
not a constant pool address. I couldn't think of a good way of fixing
this within the current framework. For example, if we set the dummy
symbol's CONSTANT_POOL_ADDRESS_P bit, the backend might pass the
symbol to functions like get_pool_mode(), which would then abort.
-> One alternative would be to add a target hook. It could take a
mode as argument and return a bitset of supported addressing types:
reg + reg, constant pool address, indirect memref, etc. Maybe we
could have a default implementation that does something akin to
init_reload.
Anyway, in terms of output code, patch 1 seems to be a strict
improvement for everything except s390. That's including mcore,
since there's an improvement to one test and no change in the others.
But before trying to fix the mcore & s390 problems, I thought I'd see
whether the general idea is OK. What do you think?
Moving on to patch 2... it only differed from patch 1 on two targets:
sh64-elf (for which it seems to be an improvement) and sparc-linux-gnu,
for which it was a regression. Diffs for both attached.
Personally, I slightly prefer patch 2 to patch 1. Giving the symbol_ref
reload a cost of 6 units seems more in keeping with the surrounding code.
So, I tried to track down the source of the sparc problem.
We had insns like:
(set (reg:SF %fx) (const_double:SF 0))
which matched *movsf_insn_novis. The best two alternatives were r <- G
and f <- m, with the following costs:
CVS Patch 1 Patch 2
(1) r <- G 9 9 9
(6) f <- m 6 8 12
The cost of alternative (1) seems a little low to me since the output
reload has to be done through memory. If we count it as two reloads
instead of one, using:
#ifdef SECONDARY_MEMORY_NEEDED
if (GET_CODE (operand) == REG
&& REGNO (operand) < FIRST_PSEUDO_REGISTER
&& this_alternative[i] != NO_REGS
&& (SECONDARY_MEMORY_NEEDED
(this_alternative[i],
REGNO_REG_CLASS (REGNO (operand)),
GET_MODE (operand))))
losers++;
#endif
then the results for patch 2 are the same as for patch 1 on sparc.
I've attached this modified version of patch 2 as patch 3. I tried
running it through all targets that define SECONDARY_MEMORY_NEEDED,
including those that accept symbol_ref addresses. Aside from s390,
it seems to be a strict improvement compared with CVS and patch 2.
Diffs attached once again. ;)
FWIW, patch 2 was bootstrapped & regression tested on various
mips targets. But this isn't really an RFA because of the s390
& mcore issues.
Richard
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: stress.c
URL: <https://gcc.gnu.org/pipermail/gcc/attachments/20030825/02b3fc2e/attachment.c>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: patch-1.diff
Type: text/x-patch
Size: 2228 bytes
Desc: not available
URL: <https://gcc.gnu.org/pipermail/gcc/attachments/20030825/02b3fc2e/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: patch-2.diff
Type: text/x-patch
Size: 2126 bytes
Desc: not available
URL: <https://gcc.gnu.org/pipermail/gcc/attachments/20030825/02b3fc2e/attachment-0001.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: patch-3.diff
Type: text/x-patch
Size: 2746 bytes
Desc: not available
URL: <https://gcc.gnu.org/pipermail/gcc/attachments/20030825/02b3fc2e/attachment-0002.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: patch-effects.tar.bz2
Type: application/x-bzip2
Size: 59069 bytes
Desc: not available
URL: <https://gcc.gnu.org/pipermail/gcc/attachments/20030825/02b3fc2e/attachment.bz2>
More information about the Gcc
mailing list