On 12/9/24 7:13 AM, Georg-Johann Lay wrote:
This patch adds a new target hook that allows to chose
a non-generic named address-space for compiler generated
lookup tables.
The purpose is that there are cases (on avr namely) where
the generic address space is sub-optimal because it must
put .rodata in RAM. With this hook it is possible to
chose an address space that's better suited, like the
__flash address space that allocates to .progmem.data which
resides in flash.
The patch passes without regressions on avr.
On x86_64, it bootstraps and tests without regressions.
Ok for trunk?
Johann
p.s. The feature has been discussed in the lists before,
and in all discussions I failed in getting across why a
different address space is needed. Some background:
1) On AVR, you cannot just put data in a different section
without also using different instructions to access it.
In general a different section also requires different
address registers and different addressing modes and
different instructions.
2) It is *not* possible to do this during linker relaxation.
You cannot just change register allocation and address registers
in the linker. You cannot just replace a 16-bit register like
X or Y by a 24-bit address that lives in Z (lower 16 bits) and
in some SFR (upper 8 bits).
3) You cannot just put all static storage read-only data into
a different address space. For example, it is perfectly fine
for a C/C++ code to define a variable in static storage and
access it in assembly code. The assembly code must know the
address space of the symbol, or otherwise the code is wrong.
4) From 3) it follows that you can only change the address space
of an object when it is hidden from the user, i.e. the compiler
is building the object and has control over all accesses, and
there's no way the user can get a reference to the object.
Understood. Hopefully I wasn't the one missing the point earlier. It
seems pretty clear to me. Essentially for these special cases we have
enough information to know that moving the data to a different part of
the address space is desirable and how to adjust the generated code
appropriately. We can't necessarily do it in general, but for compiler
generated tables it's possible.
To date, there are only 2 lookup tables generated by GCC that
fit these criteria:
A) CSWTCH tables from tree-switch-conversion.cc.
B) crc_table_for_* tables from gimple-crc-optimization.cc.
Though B) may increase the code size by quite a lot. For example,
size of gcc.dg/torture/crc-2.c will increase by more than 1500%
(and even more when a 24-bit address-space is required). The
CRC optimizations uses some builtin magic, so it's unclear where
and how to introduce a different address space.
--
Allow target to chose address-space for artificial rodata.
gcc/
* coretypes.h (enum artificial_rodata): New enum type.
* doc/tm.texi: Rebuild.
* doc/tm.texi.in (TARGET_ADDR_SPACE_FOR_ARTIFICIAL_RODATA):
New hook.
* target.def (addr_sapce.for_artificial_rodata): New DEFHOOK.
* targhooks.cc (default_addr_space_convert): New function.
* targhooks.h (default_addr_space_convert): New prototype.
* tree-switch-conversion.cc (build_one_array) <value_type>:
Set type_quals address-space according to
targetm.addr_space.for_artificial_rodata().
* config/avr/avr.cc (avr_rodata_in_flash_p): Move up.
(TARGET_ADDR_SPACE_FOR_ARTIFICIAL_RODATA): Define to...
(avr_addr_space_for_artificial_rodata): ...this new function.
artificial-rodata.diff
r *up)
}
+static bool
+avr_rodata_in_flash_p ()
Function comment. I realize you just moved it around, but you might as
well go ahead and fix the missing comment while you're at it.
Otherwise it looks sensible to me. Ok for the trunk after a fresh test
cycle.
Jeff
