[In CCing Richard Henderson]
Denis Chertykov schrieb:
2011/6/10 Georg-Johann Lay <a...@gjlay.de>:
Then I have a question on spill failures:
There is PR46278, an optimization flaw that goes as follows:
The avr BE defines fake addressing mode X+const that has to be written
down in asm as
X += const
a = *X
X -= const
The comment says that this is just to cover a corner case, however the
new register allocator uses this case more often or even greedily.
There is no way to describe the cost for such an access, and as X has
lower prologue/epilogue cost than Y, X is preferred over Y often.
In 4.7, I see that flaw less often than in 4.5. However, I think the
best way is not to lie at the register allocator and not to supply a
fake instruction like that.
So I started to work on a fix. The changes in avr.h are:
* config/avr/avr.h (BASE_REG_CLASS): Remove.
(REG_OK_FOR_BASE_NOSTRICT_P): Remove.
(REG_OK_FOR_BASE_STRICT_P): Remove.
(MODE_CODE_BASE_REG_CLASS): New Define.
(REGNO_MODE_CODE_OK_FOR_BASE_P): New Define.
The macros REGNO_MODE_CODE_OK_FOR_BASE_P and MODE_CODE_BASE_REG_CLASS
allow a better, fine grained control over addressing modes for each
hard register and allow to support X without fake instructions. The
code quality actually improves, but I see a new spill failure for the
test case
* gcc.c-torture/compile/950612-1.c
On the one hand, that test case has heavy long long use and is no
"real world code" to run on AVR. On the other hand, I don't think
trading code quality here against ICE there is a good idea.
What do you think about that? As I have no idea how to fix a spill
failure in the backend, I stopped working on a patch.
Ohhh. It's a most complicated case for GCC for AVR.
So you think is is pointless/discouraged to give a more realistic
description of AVR addressing be means of MODE_CODE_BASE_REG_CLASS
(instead of BASE_REG_CLASS) resp. REGNO_MODE_CODE_OK_FOR_BASE_P?
Look carefully at `out_movqi_r_mr'.
There are even two fake addressing modes:
1. [Y + infinite-dslacement];
2. [X + (0...63)].
Yes, I know. The first is introduced by avr_legitimate_address_p and the
second appears to be artifact of LEGITIMIZE_RELOAD_ADDRESS.
The changes are basically MODE_CODE_BASE_REG_CLASS (introduced in 4.2)
and a rewrite of avr_legitimate_address_p. The changes aim at a better
addressing for X and to minimize fake addresses.
I have spent a many hours (days, months) to debug GCC (especially avr port
and reload) for right addressing modes.
I have stopped on this code.
AVR have a limited memory addressing and GCC can't handle it in native form.
Because of that I have supported a fake adddressing modes.
I assume the code is from prior to 4.2 when
REGNO_MODE_CODE_OK_FOR_BASE_P and MODE_CODE_BASE_REG_CLASS had not been
available so that supporting X required some hacking.
All that would still be fine; however the new register allocator leads
to code that noone would accept. Accessing a structure through a pointer
is not uncommon, not even on AVR. So if Z is used for, say accessing
flash, X appears to be the best register.
The shortcoming in GCC is that there is no way to give costs of
addressing (TARGET_ADDRESS_COST does different things).
So take a look what avr-gcc compiles here:
http://gcc.gnu.org/bugzilla/attachment.cgi?id=22242
I saw similar complains in forums on the web.
(Richard Henderson have a different opinion: GCC can, AVR port can't)
What does he mean with that?
IMHO that three limited pointer registers is not enough for C compiler.
Even more with frame pointer it's only two and X is a very limited.
The current implementation has several oddities like
* allowing SUBREGs in avr-legitimate_address_p
* changing BASE_REG_CLASS on the fly (by means of reload_completed)
isn't that supposed to cause trouble?
1. [Y + infinite-dslacement] it's helper for reload addressing.
For addressing too long local/spilled variable. (Y + more-than-63)
2. [X + (0...63)] for another one "normal" pointer address.
____________________
Then I observed trouble with DI patterns during libgcc build and had
to remove
* "zero_extendqidi2"
* "zero_extendhidi2"
* "zero_extendsidi2"
These are "orphan" insns: they deal with DI without having movdi
support so I removed them.
This seems strange for me.
As far as I know, to support a mode a respective mov insn is needed,
which is not the case for DI. I don't know the exact rationale behind
that (reloading?), just read is on gcc list by Ian Taylor (and also that
it is stronly discouraged to have more than one mov insn per mode).
So if the requirement to have mov insn is dropped and without the burden
to implement movdi, it would be rather easy to implement adddi3 and
subdi3 for avr...
Johann
Denis.
