Am 27.01.25 um 16:19 schrieb Richard Sandiford:
Georg-Johann Lay <a...@gjlay.de> writes:
Am 24.01.25 um 08:18 schrieb Richard Biener:
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.

No.

That hook only returns a section *name*, not a section object.
So you cannot return an unnamed section, which by definition, don't
have a section name.

Perhaps I misunderstand what you mean, but select_section does return
a "section *".

Hi Richard,

returning a custom noswitch section in targetm.asm_out.select_section
does not work.

Reason is that varasm.cc::get_variable_section has its own ideas how
noswitch / unnamed sections should work and bypasses the result
from the target hook.

I tried to set DECL_COMMON (decl) = 0; and also
DECL_COMMON (decl) = 1; but neither of which does work.


On the point about -fdata-sections, I think avr's unique_section
should be a no-op for these variables.  That should ensure that
the compiler doesn't set DECL_SECTION_NAME itself.  (I suppose
that source-code attributes that set DECL_SECTION_NAME should be
diagnosed as incompatible with the io attribute.)

The avr backend rejects initializers for respective decls since
initializers don't make sense for the attribute.  Outcome is that
varasm puts the decl into .bss or .comm and ignores anything
from targetm.asm_out.select_section.

FWIW, I agree with Richard that a hook to bypass most of
assemble_variable doesn't feel like the right abstraction.

So would you please propose something that would be approved
*and* will work.  assemble_variable() calls a zoo of hook functions
and macros that cannot be customized since they don't provide
enough context.

I attached a patch of the targetm.asm_out.select_section
proposal for reference.  Can you please point out what I
am doing wrong, or what other hooks are eligible to implement
such a feature.

The avr testsuite has already some test cases (all of which are
currently ICEing due to abusing TLS).

$ make -k check-gcc RUNTESTFLAGS="--target_board=atmega128-sim avr.exp=pr112952*.c "

Johann
diff --git a/gcc/config/avr/avr.cc b/gcc/config/avr/avr.cc
index 656d3e7389b..f264ac22688 100644
--- a/gcc/config/avr/avr.cc
+++ b/gcc/config/avr/avr.cc
@@ -222,6 +222,8 @@ avr_arch_id avr_arch_index;
    avr_asm_select_section, but it must not be local there because of GTY.  */
 static GTY(()) section *progmem_section[ADDR_SPACE_COUNT];
 
+static GTY(()) section *my_noswitch_section;
+
 /* Condition for insns/expanders from avr-dimode.md.  */
 bool avr_have_dimode = true;
 
@@ -11930,7 +11932,12 @@ avr_asm_init_sections (void)
     readonly_data_section->unnamed.callback = avr_output_data_section_asm_op;
   data_section->unnamed.callback = avr_output_data_section_asm_op;
   bss_section->unnamed.callback = avr_output_bss_section_asm_op;
-  tls_comm_section->noswitch.callback = avr_output_addr_attrib;
+
+  extern section* get_noswitch_section (unsigned int tree,
+					noswitch_section_callback callback);
+  unsigned flags = SECTION_NOSWITCH;
+  my_noswitch_section = get_noswitch_section (flags,
+					      avr_output_addr_attrib);
 }
 
 
@@ -12159,9 +12166,8 @@ avr_encode_section_info (tree decl, rtx rtl, int new_decl_p)
 		 other way than making use of tls_comm_section.  As we are
 		 using that section anyway, also use it in the public case.  */
 
-	      DECL_COMMON (decl) = 1;
+	      DECL_COMMON (decl) = 0;
 	      set_decl_section_name (decl, (const char *) nullptr);
-	      set_decl_tls_model (decl, (tls_model) 2);
 	    }
 	}
     }
@@ -12209,6 +12215,11 @@ avr_encode_section_info (tree decl, rtx rtl, int new_decl_p)
 static section *
 avr_asm_select_section (tree decl, int reloc, unsigned HOST_WIDE_INT align)
 {
+  if (lookup_attribute ("io", DECL_ATTRIBUTES (decl))
+      || lookup_attribute ("io_low", DECL_ATTRIBUTES (decl))
+      || lookup_attribute ("address", DECL_ATTRIBUTES (decl)))
+    return my_noswitch_section;
+
   section *sect = default_elf_select_section (decl, reloc, align);
 
   if (decl && DECL_P (decl)
diff --git a/gcc/varasm.cc b/gcc/varasm.cc
index 6d93fe97d7b..a29f53a7d54 100644
--- a/gcc/varasm.cc
+++ b/gcc/varasm.cc
@@ -266,7 +266,7 @@ get_unnamed_section (unsigned int flags, void (*callback) (const char *),
 
 /* Return a SECTION_NOSWITCH section with the given fields.  */
 
-static section *
+section *
 get_noswitch_section (unsigned int flags, noswitch_section_callback callback)
 {
   section *sect;

Reply via email to