Am 23.01.25 um 14:58 schrieb Richard Biener:
On Thu, Jan 23, 2025 at 2:23 PM Georg-Johann Lay <a...@gjlay.de> wrote:
Hi, this is Ping #2 for a patch from 2024.
It adds a new target hook that allows to output
assembly code for a VAR_DECL in a custom way.
The default action is an obvious no-op,
i.e. assemble_variable() behaves like before.
I tried to understand the AVR part of the series - there I fail to see
what exactly special handling "io" and friends requires and how
that was made work with the TLS noswitch section.
The "io" and similar attributes allow to access I/O locations
with a special instruction, where the location is only known
at link time. They also allow to supply the address like
"iovar attribute((io(0x10))", and in that case the compiler only defines
a symbol like
.global iovar
iovar=0x10
i.e. does not allocate memory, and the attributes refuse initializers.
One way to output asm like that is by means of the output callback
of a noswitch section. However, the middle-end scrubs custom
noswitch sections for some reason I don't know, so the avr backend
abused tls_comm_section's callback.
This is a hack of course, and it runs into ICE in
varasm.cc::assemble_variable()
/* Emulated TLS had better not get this far. */
gcc_checking_assert (targetm.have_tls || !DECL_THREAD_LOCAL_P (decl));
The new hook puts the feature on solid grounds.
The attributes should work irrespective of
-f[no-]common -f[no-]data-sections. When you read through
(or debug through) varasm.cc, you'll notice that there is
currently no way to support a custom asm output for a VAR_DECL
irrespective of -f[no-]common -f[no-]data-sections.
I do not think the sentence the middle-end doesn't allow custom
NOSWITCH sections is correct (it would be a matter of exporting
get_noswitch_section), but I also don't exactly see how
"io" and friends require a NOSWITCH.
It was (ab)used because noswitch sections have a callback that can
output the stuff, which doesn't work for named sections since that
may depend on -f[no-]common -f[no-]data-sections.
You can of course set up a custom noswitch section in the backend
and attach it to a decl, but the middle-end will override that, so
custom noswitch section is futile. I don't remember where the
custom noswitch section is scrubbed, and I don't know what's the
purpose of that, and whether some parts of the compiler rely
on that behavior.
It appeared way more clean to add a new target hook with obvious
semantics, and an obvious no-op as its default behavior, than to
hack the middle-end's treatment of custom noswitch sections.
As I said, I don't know the rationale for why custom noswitch
sections are scrubbed and why that is required.
The implementation of the new hook is basically a complete override
of assemble_variable which IMO is bad style at least. The missing
requirements for "io" and friends should instead be made available
by other means (for example more targeted hooks).
Can you propose something more specific? varasm is quite a mess...
and IMHO the hook is clear and straight forward at least.
The placement of the hook is such that TREE_ASM_WRITTEN,
flag_syntax_only, notice_global_symbol, dont_output_data and
many more work as expected and hooks like targetm.encode_section_info
are not bypassed and work like before.
Johann
That said, I can see how the patch works and doesn't affect anything
besides AVR.
Richard.
This hook is needed in the avr backend to properly implement
some variable attributes. The avr part has already been approved.
The patch is bootstrapped, and it tests without new regressions.
Ok for trunk?
Johann
--
Original submission
https://gcc.gnu.org/pipermail/gcc-patches/2025-January/673459.html
Ping #1
https://gcc.gnu.org/pipermail/gcc-patches/2025-January/673459.html
avr part and approval
https://gcc.gnu.org/pipermail/gcc-patches/2024-December/672051.html
https://gcc.gnu.org/pipermail/gcc-patches/2024-December/672084.html
--
Add new target hook TARGET_ASM_VARIABLE.
This patch adds a new target hook that allows the backend to asm output
a variable definition in its own way. This hook is needed because
varasm.cc imposes a very restrictive layout for all variable definitions
which will be basically ELF style (on ELF targets as least). To date,
there is no way for a backend to output a variable definition in a
different way.
This hook is required by the avr backend when it outputs definitions
for variables defined with the "io", "io_low" or "address" attribute that
don't follow ELF style. These attributes are basically symbol definitions
of the form
.global var_io
var_io = 32
with some additional assertions.
gcc/
* target.def (TARGET_ASM_OUT) <variable>: Add new DEFHOOK.
* targhooks.cc (default_asm_out_variable): New function.
* targhooks.h (default_asm_out_variable): New prototype.
* doc/tm.texi.in (TARGET_ASM_VARIABLE): Place hook documentation.
* doc/tm.texi: Rebuild.
* varasm.cc (assemble_variable): Call targetm.asm_out.variable
in order to allow the backend to output a variable definition
in its own style.
