On Thu, Jan 23, 2025 at 4:53 PM Georg-Johann Lay <a...@gjlay.de> wrote: > > 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. > > > > 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. > > A noswitch section is one way to go. > > However, there is no way to attach a noswitch section to a VAR_DECL.
Hmm, I don't know varasm.cc by heart either but there's the select_section hook invoked by get_variable_section () which looks like it could do the trick. But as for IO and friends - you say those are just .global x = Y and thus always DECL_EXTERN? Aka in no actual section? So they are some kind of alias for a specific address (eventually determined by the linker)? That is, if it's not allocated I wonder why we end up doing assemble_variable at all (IIRC refs to externs are output differently). > noswitch sections are selected by flags like DECL_COMMON or > DECL_INITIAL and command line options like -f[no-]common and > -f[no-]data-sections. > > In the case of tls_comm_section, the section isn't attached to > a decl, but by means of flags like: > > set_decl_section_name (decl, (const char *) nullptr); > set_decl_tls_model (decl, (tls_model) 2); > > Anyway, all the noswitch stuff was only a hack since it was > the only way to do in in the backend alone. > A proper implementation of the feature required some changes in > varasm, and a hook like proposed seems a clear and sound approach. > > > The implementation of the new hook is basically a complete override > > of assemble_variable which IMO is bad style at least. The missing > > So what's a better approach? Hookizing assemble_variable altogether, > and putting the current assemble_variable code into the hook default? > > Johann > > > requirements for "io" and friends should instead be made available > > by other means (for example more targeted hooks). > > > > 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.
