On Thu, Aug 7, 2025 at 12:00 AM Richard Biener
<richard.guent...@gmail.com> wrote:
>
> On Thu, Aug 7, 2025 at 8:37 AM Andrew Pinski <quic_apin...@quicinc.com> wrote:
> >
> > Like r0-46282-gf41115930523b3, but this time for normal variables
> > rather than the constant pool.
> > With the simple C++ code on ia32:
> > ```
> > std::initializer_list<long double> a = {0.3l};
> > ```
> > GCC currently produces:
> > ```
> > .section .rodata.cst16,"aM",@progbits,16
> > .align 16
> > .type _ZGR1a_, @object
> > .size _ZGR1a_, 12
> > _ZGR1a_:
> > .long -1717986918
> > .long -1717986919
> > .long 16381
> > .text
> > ```
> > Notice how the mereable section is not being aligned at the end to
> > 16 bytes.
> > This was exactly the same issue as reported in
> > https://sourceware.org/legacy-ml/binutils/2002-11/msg00615.html
> > except this time for DECLs which mergeable rather than the constant pool
> > (which was fixed by r0-46282-gf41115930523b3).
> >
> > So we need to ensure the variables in mergable sections are filed
> > correctly for the rest of the section.
> >
> > Note this showed up more with gcn after r16-2595-gf1c80147641783 since
> > more decls are done in mergable sections without the same size as the
> > alignment.
> >
> > Bootstrapped and tested on x86_64-linux-gnu.
> >
> > PR middle-end/121394
> >
> > gcc/ChangeLog:
> >
> > * varasm.cc (assemble_variable_contents): Ensure mergeable sections
> > get padded for each decl.
> >
> > Signed-off-by: Andrew Pinski <quic_apin...@quicinc.com>
> > ---
> > gcc/varasm.cc | 6 ++++++
> > 1 file changed, 6 insertions(+)
> >
> > diff --git a/gcc/varasm.cc b/gcc/varasm.cc
> > index 000ad9e26f6..7b04b82c1a5 100644
> > --- a/gcc/varasm.cc
> > +++ b/gcc/varasm.cc
> > @@ -2475,6 +2475,12 @@ assemble_variable_contents (tree decl, const char
> > *name,
> > else
> > /* Leave space for it. */
> > assemble_zeros (tree_to_uhwi (DECL_SIZE_UNIT (decl)));
> > + /* Make sure all decls in SECTION_MERGE sections have proper size.
> > */
> > + if (in_section
> > + && (in_section->common.flags & SECTION_MERGE)
> > + && tree_fits_uhwi_p (DECL_SIZE (decl))
> > + && DECL_ALIGN (decl) > tree_to_uhwi (DECL_SIZE (decl)))
>
> Isn't that a wrong test? Consider 16 byte alignment and a 24 byte object.
>
> Is the concern only aligned but packed decls btw? I'd have expected the
> size to be always a multiple of the alignment otherwise.
There is a better way of implementing this.
/* For mergeable section, make sure the section is zero filled up to
the entity size of the section */
if (in_section
&& (in_section->common.flags & SECTION_MERGE)
&& tree_fits_uhwi_p (DECL_SIZE (decl))
&& (in_section->common.flags & SECTION_ENTSIZE) > tree_to_uhwi
(DECL_SIZE (decl)))
assemble_zeros ((in_section->common.flags & SECTION_ENTSIZE) -
tree_to_uhwi (DECL_SIZE (decl)));
Note (in_section->common.flags & SECTION_ENTSIZE) contains the entity
size of the section. and will be zero for the mergeable string
section.
I will be combining this with another patch which I will be doing
which removes the alignment requirement for the section, just the size
needs to be small to be placed into the mergeable constant section.
Thanks,
Andrew Pinski
>
> Richard.
>
> > + assemble_align (DECL_ALIGN (decl));
> > targetm.asm_out.decl_end ();
> > }
> > }
> > --
> > 2.43.0
> >
