https://gcc.gnu.org/g:8fa1be51876b8dae6173ae5d6493b3d4894f7d18
commit 8fa1be51876b8dae6173ae5d6493b3d4894f7d18
Author: Thomas Schwinge <tschwi...@baylibre.com>
Date: Thu Feb 13 00:35:31 2025 +0100
nvptx: Sanity-check 'init_frag' state
The 'init_frag' machinery is used by 'nvptx_assemble_integer' (via
'TARGET_ASM_INTEGER'), 'nvptx_output_skip' (via 'ASM_OUTPUT_SKIP'),
'nvptx_output_ascii' (via 'ASM_OUTPUT_ASCII'). But, it's not obvious that
these are called only when that machinery is active (and in a consistent
state), which it only is in 'nvptx_output_aligned_decl' (via
'ASM_OUTPUT_ALIGNED_DECL_COMMON', or 'ASM_OUTPUT_ALIGNED_DECL_LOCAL'), or in
'nvptx_assemble_undefined_decl' (via 'TARGET_ASM_ASSEMBLE_UNDEFINED_DECL'),
or
within a region started by 'nvptx_assemble_decl_begin' (via
'nvptx_asm_declare_constant_name' (via 'TARGET_ASM_DECLARE_CONSTANT_NAME'),
or
via 'nvptx_declare_object_name' (via 'ASM_DECLARE_OBJECT_NAME')) and ended
by
'nvptx_assemble_decl_end' (via 'TARGET_ASM_DECL_END').
And indeed, in a GCC/nvptx offloading configuration, we then find that
'nvptx_output_skip' (via 'ASM_OUTPUT_SKIP') is getting called in
inconsistent
'init_frag' state, in 'gcc/varasm.cc', via 'assemble_zeros', from
'output_object_block', to "Move to the object's offset, padding with zeros".
Supposedly, this didn't cause any damage, but we now handle it explicitly.
(..., and the question remains whether such "padding" etc. shouldn't
actually
be attempted for targets like nvptx.)
gcc/
* config/nvptx/nvptx.cc (init_frag): New 'bool active' member.
(output_init_frag, nvptx_assemble_value, nvptx_assemble_integer)
(nvptx_output_skip, nvptx_assemble_decl_begin)
(nvptx_assemble_decl_end): Sanity-check its state.
(cherry picked from commit 79cb26298c76f1360f1aada26863f011c4decc34)
Diff:
---
gcc/ChangeLog.omp | 8 ++++++++
gcc/config/nvptx/nvptx.cc | 26 ++++++++++++++++++++++++++
2 files changed, 34 insertions(+)
diff --git a/gcc/ChangeLog.omp b/gcc/ChangeLog.omp
index 2bc4343b2235..c998343683e4 100644
--- a/gcc/ChangeLog.omp
+++ b/gcc/ChangeLog.omp
@@ -3,6 +3,14 @@
Backported from trunk:
2025-02-14 Thomas Schwinge <tschwi...@baylibre.com>
+ * config/nvptx/nvptx.cc (init_frag): New 'bool active' member.
+ (output_init_frag, nvptx_assemble_value, nvptx_assemble_integer)
+ (nvptx_output_skip, nvptx_assemble_decl_begin)
+ (nvptx_assemble_decl_end): Sanity-check its state.
+
+ Backported from trunk:
+ 2025-02-14 Thomas Schwinge <tschwi...@baylibre.com>
+
* config/nvptx/nvptx.cc (nvptx_output_skip): Clarify case of
no or incomplete initializer.
diff --git a/gcc/config/nvptx/nvptx.cc b/gcc/config/nvptx/nvptx.cc
index ef867daba3c5..25f51c9420b7 100644
--- a/gcc/config/nvptx/nvptx.cc
+++ b/gcc/config/nvptx/nvptx.cc
@@ -2331,6 +2331,7 @@ static struct
out. */
unsigned size; /* Fragment size to accumulate. */
unsigned offset; /* Offset within current fragment. */
+ bool active; /* Whether this machinery is active. */
bool started; /* Whether we've output any initializer. */
} init_frag;
@@ -2341,6 +2342,8 @@ static struct
static void
output_init_frag (rtx sym)
{
+ gcc_checking_assert (init_frag.active);
+
fprintf (asm_out_file, init_frag.started ? ", " : " = { ");
unsigned HOST_WIDE_INT val = init_frag.val;
@@ -2372,6 +2375,8 @@ output_init_frag (rtx sym)
static void
nvptx_assemble_value (unsigned HOST_WIDE_INT val, unsigned size)
{
+ gcc_checking_assert (init_frag.active);
+
bool negative_p
= val & (HOST_WIDE_INT_1U << (HOST_BITS_PER_WIDE_INT - 1));
@@ -2404,6 +2409,8 @@ nvptx_assemble_value (unsigned HOST_WIDE_INT val,
unsigned size)
static bool
nvptx_assemble_integer (rtx x, unsigned int size, int ARG_UNUSED (aligned_p))
{
+ gcc_checking_assert (init_frag.active);
+
HOST_WIDE_INT val = 0;
switch (GET_CODE (x))
@@ -2446,6 +2453,17 @@ nvptx_assemble_integer (rtx x, unsigned int size, int
ARG_UNUSED (aligned_p))
void
nvptx_output_skip (FILE *, unsigned HOST_WIDE_INT size)
{
+ gcc_checking_assert (in_section == data_section
+ || in_section == text_section);
+
+ if (!init_frag.active)
+ /* We're in the 'data_section' or 'text_section', outside of an
+ initializer context ('init_frag'). There's nothing to do here:
+ in PTX, there's no concept of an assembler's "location counter",
+ "current address", "dot symbol" ('.') that might need padding or
+ aligning. */
+ return;
+
/* Finish the current fragment, if it's started. */
if (init_frag.offset)
{
@@ -2522,6 +2540,8 @@ nvptx_assemble_decl_begin (FILE *file, const char *name,
const char *section,
const_tree type, HOST_WIDE_INT size, unsigned align,
bool undefined = false)
{
+ gcc_checking_assert (!init_frag.active);
+
bool atype = (TREE_CODE (type) == ARRAY_TYPE)
&& (TYPE_DOMAIN (type) == NULL_TREE);
@@ -2549,6 +2569,8 @@ nvptx_assemble_decl_begin (FILE *file, const char *name,
const char *section,
elt_size |= GET_MODE_SIZE (elt_mode);
elt_size &= -elt_size; /* Extract LSB set. */
+ init_frag.active = true;
+
init_frag.size = elt_size;
/* Avoid undefined shift behavior by using '2'. */
init_frag.mask = ((unsigned HOST_WIDE_INT)2
@@ -2580,10 +2602,14 @@ nvptx_assemble_decl_begin (FILE *file, const char
*name, const char *section,
static void
nvptx_assemble_decl_end (void)
{
+ gcc_checking_assert (init_frag.active);
+
if (init_frag.offset)
/* This can happen with a packed struct with trailing array member. */
nvptx_assemble_value (0, init_frag.size - init_frag.offset);
fprintf (asm_out_file, init_frag.started ? " };\n" : ";\n");
+
+ init_frag.active = false;
}
/* Output an uninitialized common or file-scope variable. */
