Skip to content

Commit

Permalink
aarch64: fix warning emission for ABI break since GCC 9.1
Browse files Browse the repository at this point in the history 
While looking at PR 105549, which is about fixing the ABI break
introduced in GCC 9.1 in parameter alignment with bit-fields, we
noticed that the GCC 9.1 warning is not emitted in all the cases where
it should be.  This patch fixes that and the next patch in the series
fixes the GCC 9.1 break.

We split this into two patches since patch #2 introduces a new ABI
break starting with GCC 13.1.  This way, patch #1 can be back-ported
to release branches if needed to fix the GCC 9.1 warning issue.

The main idea is to add a new global boolean that indicates whether
we're expanding the start of a function, so that aarch64_layout_arg
can emit warnings for callees as well as callers.  This removes the
need for aarch64_function_arg_boundary to warn (with its incomplete
information).  However, in the first patch there are still cases where
we emit warnings were we should not; this is fixed in patch #2 where
we can distinguish between GCC 9.1 and GCC.13.1 ABI breaks properly.

The fix in aarch64_function_arg_boundary (replacing & with &&) looks
like an oversight of a previous commit in this area which changed
'abi_break' from a boolean to an integer.

We also take the opportunity to fix the comment above
aarch64_function_arg_alignment since the value of the abi_break
parameter was changed in a previous commit, no longer matching the
description.

2022-11-28  Christophe Lyon  <christophe.lyon@arm.com>
	    Richard Sandiford  <richard.sandiford@arm.com>

gcc/ChangeLog:

	* config/aarch64/aarch64.cc (aarch64_function_arg_alignment): Fix
	comment.
	(aarch64_layout_arg): Factorize warning conditions.
	(aarch64_function_arg_boundary): Fix typo.
	* function.cc (currently_expanding_function_start): New variable.
	(expand_function_start): Handle
	currently_expanding_function_start.
	* function.h (currently_expanding_function_start): Declare.

gcc/testsuite/ChangeLog:

	* gcc.target/aarch64/bitfield-abi-warning-align16-O2.c: New test.
	* gcc.target/aarch64/bitfield-abi-warning-align16-O2-extra.c: New
	test.
	* gcc.target/aarch64/bitfield-abi-warning-align32-O2.c: New test.
	* gcc.target/aarch64/bitfield-abi-warning-align32-O2-extra.c: New
	test.
	* gcc.target/aarch64/bitfield-abi-warning-align8-O2.c: New test.
	* gcc.target/aarch64/bitfield-abi-warning.h: New test.
	* g++.target/aarch64/bitfield-abi-warning-align16-O2.C: New test.
	* g++.target/aarch64/bitfield-abi-warning-align16-O2-extra.C: New
	test.
	* g++.target/aarch64/bitfield-abi-warning-align32-O2.C: New test.
	* g++.target/aarch64/bitfield-abi-warning-align32-O2-extra.C: New
	test.
	* g++.target/aarch64/bitfield-abi-warning-align8-O2.C: New test.
	* g++.target/aarch64/bitfield-abi-warning.h: New test.
  • Loading branch information
Christophe Lyon committed Jan 12, 2023
1 parent b073f2b commit 3df1a11
Show file tree
Hide file tree
Showing 15 changed files with 1,132 additions and 7 deletions.
28 changes: 21 additions & 7 deletions gcc/config/aarch64/aarch64.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7536,9 +7536,9 @@ aarch64_vfp_is_call_candidate (cumulative_args_t pcum_v, machine_mode mode,
/* Given MODE and TYPE of a function argument, return the alignment in
bits. The idea is to suppress any stronger alignment requested by
the user and opt for the natural alignment (specified in AAPCS64 \S
4.1). ABI_BREAK is set to true if the alignment was incorrectly
calculated in versions of GCC prior to GCC-9. This is a helper
function for local use only. */
4.1). ABI_BREAK is set to the old alignment if the alignment was
incorrectly calculated in versions of GCC prior to GCC-9. This is
a helper function for local use only. */

static unsigned int
aarch64_function_arg_alignment (machine_mode mode, const_tree type,
Expand Down Expand Up @@ -7614,11 +7614,24 @@ aarch64_layout_arg (cumulative_args_t pcum_v, const function_arg_info &arg)
if (pcum->aapcs_arg_processed)
return;

bool warn_pcs_change
= (warn_psabi
&& !pcum->silent_p
&& (currently_expanding_function_start
|| currently_expanding_gimple_stmt));

unsigned int alignment
= aarch64_function_arg_alignment (mode, type, &abi_break);
gcc_assert (!alignment || abi_break < alignment);

pcum->aapcs_arg_processed = true;

pure_scalable_type_info pst_info;
if (type && pst_info.analyze_registers (type))
{
/* aarch64_function_arg_alignment has never had an effect on
this case. */

/* The PCS says that it is invalid to pass an SVE value to an
unprototyped function. There is no ABI-defined location we
can return in this case, so we have no real choice but to raise
Expand Down Expand Up @@ -7689,6 +7702,8 @@ aarch64_layout_arg (cumulative_args_t pcum_v, const function_arg_info &arg)
and homogenous short-vector aggregates (HVA). */
if (allocate_nvrn)
{
/* aarch64_function_arg_alignment has never had an effect on
this case. */
if (!pcum->silent_p && !TARGET_FLOAT)
aarch64_err_no_fpadvsimd (mode);

Expand Down Expand Up @@ -7753,7 +7768,7 @@ aarch64_layout_arg (cumulative_args_t pcum_v, const function_arg_info &arg)
&& (aarch64_function_arg_alignment (mode, type, &abi_break)
== 16 * BITS_PER_UNIT))
{
if (abi_break && warn_psabi && currently_expanding_gimple_stmt)
if (warn_pcs_change && abi_break)
inform (input_location, "parameter passing for argument of type "
"%qT changed in GCC 9.1", type);
++ncrn;
Expand Down Expand Up @@ -7816,7 +7831,7 @@ aarch64_layout_arg (cumulative_args_t pcum_v, const function_arg_info &arg)
int new_size = ROUND_UP (pcum->aapcs_stack_size, 16 / UNITS_PER_WORD);
if (pcum->aapcs_stack_size != new_size)
{
if (abi_break && warn_psabi && currently_expanding_gimple_stmt)
if (warn_pcs_change && abi_break)
inform (input_location, "parameter passing for argument of type "
"%qT changed in GCC 9.1", type);
pcum->aapcs_stack_size = new_size;
Expand Down Expand Up @@ -7936,14 +7951,13 @@ aarch64_function_arg_boundary (machine_mode mode, const_tree type)
unsigned int alignment = aarch64_function_arg_alignment (mode, type,
&abi_break);
alignment = MIN (MAX (alignment, PARM_BOUNDARY), STACK_BOUNDARY);
if (abi_break & warn_psabi)
if (abi_break && warn_psabi)
{
abi_break = MIN (MAX (abi_break, PARM_BOUNDARY), STACK_BOUNDARY);
if (alignment != abi_break)
inform (input_location, "parameter passing for argument of type "
"%qT changed in GCC 9.1", type);
}

return alignment;
}

Expand Down
5 changes: 5 additions & 0 deletions gcc/function.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5062,9 +5062,12 @@ stack_protect_epilogue (void)
PARMS_HAVE_CLEANUPS is nonzero if there are cleanups associated with
the function's parameters, which must be run at any return statement. */

bool currently_expanding_function_start;
void
expand_function_start (tree subr)
{
currently_expanding_function_start = true;

/* Make sure volatile mem refs aren't considered
valid operands of arithmetic insns. */
init_recog_no_volatile ();
Expand Down Expand Up @@ -5257,6 +5260,8 @@ expand_function_start (tree subr)
/* If we are doing generic stack checking, the probe should go here. */
if (flag_stack_check == GENERIC_STACK_CHECK)
stack_check_probe_note = emit_note (NOTE_INSN_DELETED);

currently_expanding_function_start = false;
}

void
Expand Down
2 changes: 2 additions & 0 deletions gcc/function.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -723,4 +723,6 @@ extern const char *current_function_name (void);

extern void used_types_insert (tree);

extern bool currently_expanding_function_start;

#endif /* GCC_FUNCTION_H */
86 changes: 86 additions & 0 deletions gcc/testsuite/g++.target/aarch64/bitfield-abi-warning-align16-O2-extra.C
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
/* { dg-do compile } */
/* { dg-options "-O2 -save-temps -Wno-narrowing" } */

#define ALIGN 16
//#define EXTRA

#include "bitfield-abi-warning.h"

/* In f1, f2, f4, f8, f16, f16p (and stdarg versions): */
/* { dg-final { scan-assembler-times "and\tw0, w2, 1" 12 { xfail *-*-* } } } */
/* In fp, f1p, f2p, f4p, f8p (and stdarg versions): */
/* { dg-final { scan-assembler-times "and\tw0, w1, 1" 10 { xfail *-*-* } } } */

/* Bitfield parameter in registers. */
/* { dg-note {parameter passing for argument of type 'S1' changed in GCC 9.1} "" { target *-*-* } 47 } f1 */
/* { dg-note {parameter passing for argument of type 'S2' changed in GCC 9.1} "" { target *-*-* } 48 } f2 */
/* { dg-note {parameter passing for argument of type 'S4' changed in GCC 9.1} "" { target *-*-* } 49 } f4 */
/* { dg-note {parameter passing for argument of type 'S8' changed in GCC 9.1} "" { target *-*-* } 50 } f8 */

/* { dg-note {parameter passing for argument of type 'Sp' changed in GCC 9.1} "" { target *-*-* } 53 } fp */
/* { dg-note {parameter passing for argument of type 'S1p' changed in GCC 9.1} "" { target *-*-* } 54 } f1p */
/* { dg-note {parameter passing for argument of type 'S2p' changed in GCC 9.1} "" { target *-*-* } 55 } f2p */
/* { dg-note {parameter passing for argument of type 'S4p' changed in GCC 9.1} "" { target *-*-* } 56 } f4p */
/* { dg-note {parameter passing for argument of type 'S8p' changed in GCC 9.1} "" { target *-*-* } 57 } f8p */

/* Bitfield call argument in registers. */
/* { dg-note {parameter passing for argument of type 'S1' changed in GCC 9.1} "" { target *-*-* } 60 } g1 */
/* { dg-note {parameter passing for argument of type 'S2' changed in GCC 9.1} "" { target *-*-* } 61 } g2 */
/* { dg-note {parameter passing for argument of type 'S4' changed in GCC 9.1} "" { target *-*-* } 62 } g4 */
/* { dg-note {parameter passing for argument of type 'S8' changed in GCC 9.1} "" { target *-*-* } 63 } g8 */

/* { dg-note {parameter passing for argument of type 'Sp' changed in GCC 9.1} "" { target *-*-* } 66 } gp */
/* { dg-note {parameter passing for argument of type 'S1p' changed in GCC 9.1} "" { target *-*-* } 67 } g1p */
/* { dg-note {parameter passing for argument of type 'S2p' changed in GCC 9.1} "" { target *-*-* } 68 } g2p */
/* { dg-note {parameter passing for argument of type 'S4p' changed in GCC 9.1} "" { target *-*-* } 69 } g4p */
/* { dg-note {parameter passing for argument of type 'S8p' changed in GCC 9.1} "" { target *-*-* } 70 } g8p */


/* Bitfield parameter in stack. */
/* { dg-note {parameter passing for argument of type 'S1' changed in GCC 9.1} "" { target *-*-* } 74 } f1_stack */
/* { dg-note {parameter passing for argument of type 'S2' changed in GCC 9.1} "" { target *-*-* } 75 } f2_stack */
/* { dg-note {parameter passing for argument of type 'S4' changed in GCC 9.1} "" { target *-*-* } 76 } f4_stack */
/* { dg-note {parameter passing for argument of type 'S8' changed in GCC 9.1} "" { target *-*-* } 77 } f8_stack */

/* { dg-note {parameter passing for argument of type 'Sp' changed in GCC 9.1} "" { target *-*-* } 80 } fp_stack */
/* { dg-note {parameter passing for argument of type 'S1p' changed in GCC 9.1} "" { target *-*-* } 81 } f1p_stack */
/* { dg-note {parameter passing for argument of type 'S2p' changed in GCC 9.1} "" { target *-*-* } 82 } f2p_stack */
/* { dg-note {parameter passing for argument of type 'S4p' changed in GCC 9.1} "" { target *-*-* } 83 } f4p_stack */
/* { dg-note {parameter passing for argument of type 'S8p' changed in GCC 9.1} "" { target *-*-* } 84 } f8p_stack */

/* Bitfield call argument in stack. */
/* { dg-note {parameter passing for argument of type 'S1' changed in GCC 9.1} "" { target *-*-* } 87 } g1_stack */
/* { dg-note {parameter passing for argument of type 'S2' changed in GCC 9.1} "" { target *-*-* } 88 } g2_stack */
/* { dg-note {parameter passing for argument of type 'S4' changed in GCC 9.1} "" { target *-*-* } 89 } g4_stack */
/* { dg-note {parameter passing for argument of type 'S8' changed in GCC 9.1} "" { target *-*-* } 90 } g8_stack */

/* { dg-note {parameter passing for argument of type 'Sp' changed in GCC 9.1} "" { target *-*-* } 93 } gp_stack */
/* { dg-note {parameter passing for argument of type 'S1p' changed in GCC 9.1} "" { target *-*-* } 94 } g1p_stack */
/* { dg-note {parameter passing for argument of type 'S2p' changed in GCC 9.1} "" { target *-*-* } 95 } g2p_stack */
/* { dg-note {parameter passing for argument of type 'S4p' changed in GCC 9.1} "" { target *-*-* } 96 } g4p_stack */
/* { dg-note {parameter passing for argument of type 'S8p' changed in GCC 9.1} "" { target *-*-* } 97 } g8p_stack */


/* Bitfield parameter in stdarg. */
/* { dg-note {parameter passing for argument of type 'S1' changed in GCC 9.1} "" { target *-*-* } 101 } f1_stdarg */
/* { dg-note {parameter passing for argument of type 'S2' changed in GCC 9.1} "" { target *-*-* } 102 } f2_stdarg */
/* { dg-note {parameter passing for argument of type 'S4' changed in GCC 9.1} "" { target *-*-* } 103 } f4_stdarg */
/* { dg-note {parameter passing for argument of type 'S8' changed in GCC 9.1} "" { target *-*-* } 104 } f8_stdarg */

/* { dg-note {parameter passing for argument of type 'Sp' changed in GCC 9.1} "" { target *-*-* } 107 } fp_stdarg */
/* { dg-note {parameter passing for argument of type 'S1p' changed in GCC 9.1} "" { target *-*-* } 108 } f1p_stdarg */
/* { dg-note {parameter passing for argument of type 'S2p' changed in GCC 9.1} "" { target *-*-* } 109 } f2p_stdarg */
/* { dg-note {parameter passing for argument of type 'S4p' changed in GCC 9.1} "" { target *-*-* } 110 } f4p_stdarg */
/* { dg-note {parameter passing for argument of type 'S8p' changed in GCC 9.1} "" { target *-*-* } 111 } f8p_stdarg */

/* Bitfield call argument in stdarg. */
/* { dg-note {parameter passing for argument of type 'S1' changed in GCC 9.1} "" { target *-*-* } 114 } g1_stdarg */
/* { dg-note {parameter passing for argument of type 'S2' changed in GCC 9.1} "" { target *-*-* } 115 } g2_stdarg */
/* { dg-note {parameter passing for argument of type 'S4' changed in GCC 9.1} "" { target *-*-* } 116 } g4_stdarg */
/* { dg-note {parameter passing for argument of type 'S8' changed in GCC 9.1} "" { target *-*-* } 117 } g8_stdarg */

/* { dg-note {parameter passing for argument of type 'Sp' changed in GCC 9.1} "" { target *-*-* } 120 } gp_stdarg */
/* { dg-note {parameter passing for argument of type 'S1p' changed in GCC 9.1} "" { target *-*-* } 121 } g1p_stdarg */
/* { dg-note {parameter passing for argument of type 'S2p' changed in GCC 9.1} "" { target *-*-* } 122 } g2p_stdarg */
/* { dg-note {parameter passing for argument of type 'S4p' changed in GCC 9.1} "" { target *-*-* } 123 } g4p_stdarg */
/* { dg-note {parameter passing for argument of type 'S8p' changed in GCC 9.1} "" { target *-*-* } 124 } g8p_stdarg */
87 changes: 87 additions & 0 deletions gcc/testsuite/g++.target/aarch64/bitfield-abi-warning-align16-O2.C
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
/* { dg-do compile } */
/* { dg-options "-O2 -save-temps -Wno-narrowing" } */

#define ALIGN 16
#define EXTRA

#include "bitfield-abi-warning.h"

/* In f1, f2, f4, f8, f16, f16p (and stdarg versions): */
/* { dg-final { scan-assembler-times "and\tw0, w2, 1" 12 { xfail *-*-* } } } */
/* In fp, f1p, f2p, f4p, f8p (and stdarg versions): */
/* { dg-final { scan-assembler-times "and\tw0, w1, 1" 10 { xfail *-*-* } } } */

/* Bitfield parameter in registers. */
/* { dg-note {parameter passing for argument of type 'S1' changed in GCC 9.1} "" { target *-*-* } 47 } f1 */
/* { dg-note {parameter passing for argument of type 'S2' changed in GCC 9.1} "" { target *-*-* } 48 } f2 */
/* { dg-note {parameter passing for argument of type 'S4' changed in GCC 9.1} "" { target *-*-* } 49 } f4 */
/* { dg-note {parameter passing for argument of type 'S8' changed in GCC 9.1} "" { target *-*-* } 50 } f8 */

/* No change in parameter passing in GCC 9.1 for lines 53-57 (fp, f1p, f2p,
f4p, f8p) (because the argument fits in a single register). Should not
warn, but aarch64_function_arg_boundary would need to take the argument size
into account as well as whether it's passed via registers or the stack. */
/* { dg-note {parameter passing for argument of type 'Sp' changed in GCC 9.1} "" { target *-*-* } 53 } fp */
/* { dg-note {parameter passing for argument of type 'S1p' changed in GCC 9.1} "" { target *-*-* } 54 } f1p */
/* { dg-note {parameter passing for argument of type 'S2p' changed in GCC 9.1} "" { target *-*-* } 55 } f2p */
/* { dg-note {parameter passing for argument of type 'S4p' changed in GCC 9.1} "" { target *-*-* } 56 } f4p */
/* { dg-note {parameter passing for argument of type 'S8p' changed in GCC 9.1} "" { target *-*-* } 57 } f8p */

/* Bitfield call argument in registers. */
/* { dg-note {parameter passing for argument of type 'S1' changed in GCC 9.1} "" { target *-*-* } 60 } g1 */
/* { dg-note {parameter passing for argument of type 'S2' changed in GCC 9.1} "" { target *-*-* } 61 } g2 */
/* { dg-note {parameter passing for argument of type 'S4' changed in GCC 9.1} "" { target *-*-* } 62 } g4 */
/* { dg-note {parameter passing for argument of type 'S8' changed in GCC 9.1} "" { target *-*-* } 63 } g8 */

/* No change in parameter passing in GCC 9.1 for lines 66-70 (gp, g1p, g2p,
g4p, g8p), no warning expected. */


/* Bitfield parameter in stack. */
/* { dg-note {parameter passing for argument of type 'S1' changed in GCC 9.1} "" { target *-*-* } 74 } f1_stack */
/* { dg-note {parameter passing for argument of type 'S2' changed in GCC 9.1} "" { target *-*-* } 75 } f2_stack */
/* { dg-note {parameter passing for argument of type 'S4' changed in GCC 9.1} "" { target *-*-* } 76 } f4_stack */
/* { dg-note {parameter passing for argument of type 'S8' changed in GCC 9.1} "" { target *-*-* } 77 } f8_stack */

/* { dg-note {parameter passing for argument of type 'Sp' changed in GCC 9.1} "" { target *-*-* } 80 } fp_stack */
/* { dg-note {parameter passing for argument of type 'S1p' changed in GCC 9.1} "" { target *-*-* } 81 } f1p_stack */
/* { dg-note {parameter passing for argument of type 'S2p' changed in GCC 9.1} "" { target *-*-* } 82 } f2p_stack */
/* { dg-note {parameter passing for argument of type 'S4p' changed in GCC 9.1} "" { target *-*-* } 83 } f4p_stack */
/* { dg-note {parameter passing for argument of type 'S8p' changed in GCC 9.1} "" { target *-*-* } 84 } f8p_stack */

/* Bitfield call argument in stack. */
/* { dg-note {parameter passing for argument of type 'S1' changed in GCC 9.1} "" { target *-*-* } 87 } g1_stack */
/* { dg-note {parameter passing for argument of type 'S2' changed in GCC 9.1} "" { target *-*-* } 88 } g2_stack */
/* { dg-note {parameter passing for argument of type 'S4' changed in GCC 9.1} "" { target *-*-* } 89 } g4_stack */
/* { dg-note {parameter passing for argument of type 'S8' changed in GCC 9.1} "" { target *-*-* } 90 } g8_stack */

/* { dg-note {parameter passing for argument of type 'Sp' changed in GCC 9.1} "" { target *-*-* } 93 } gp_stack */
/* { dg-note {parameter passing for argument of type 'S1p' changed in GCC 9.1} "" { target *-*-* } 94 } g1p_stack */
/* { dg-note {parameter passing for argument of type 'S2p' changed in GCC 9.1} "" { target *-*-* } 95 } g2p_stack */
/* { dg-note {parameter passing for argument of type 'S4p' changed in GCC 9.1} "" { target *-*-* } 96 } g4p_stack */
/* { dg-note {parameter passing for argument of type 'S8p' changed in GCC 9.1} "" { target *-*-* } 97 } g8p_stack */


/* Bitfield parameter in stdarg. */
/* { dg-note {parameter passing for argument of type 'S1' changed in GCC 9.1} "" { target *-*-* } 101 } f1_stdarg */
/* { dg-note {parameter passing for argument of type 'S2' changed in GCC 9.1} "" { target *-*-* } 102 } f2_stdarg */
/* { dg-note {parameter passing for argument of type 'S4' changed in GCC 9.1} "" { target *-*-* } 103 } f4_stdarg */
/* { dg-note {parameter passing for argument of type 'S8' changed in GCC 9.1} "" { target *-*-* } 104 } f8_stdarg */

/* Parameter passing for these should not have changed in GCC 9.1 (PR 105549).
Fortunately we warn. Note the discrepancy with lines 120-124 below: we warn
in the callee, but not in the caller. */
/* { dg-note {parameter passing for argument of type 'Sp' changed in GCC 9.1} "" { target *-*-* } 107 } fp_stdarg */
/* { dg-note {parameter passing for argument of type 'S1p' changed in GCC 9.1} "" { target *-*-* } 108 } f1p_stdarg */
/* { dg-note {parameter passing for argument of type 'S2p' changed in GCC 9.1} "" { target *-*-* } 109 } f2p_stdarg */
/* { dg-note {parameter passing for argument of type 'S4p' changed in GCC 9.1} "" { target *-*-* } 110 } f4p_stdarg */
/* { dg-note {parameter passing for argument of type 'S8p' changed in GCC 9.1} "" { target *-*-* } 111 } f8p_stdarg */

/* Bitfield call argument in stdarg. */
/* { dg-note {parameter passing for argument of type 'S1' changed in GCC 9.1} "" { target *-*-* } 114 } g1_stdarg */
/* { dg-note {parameter passing for argument of type 'S2' changed in GCC 9.1} "" { target *-*-* } 115 } g2_stdarg */
/* { dg-note {parameter passing for argument of type 'S4' changed in GCC 9.1} "" { target *-*-* } 116 } g4_stdarg */
/* { dg-note {parameter passing for argument of type 'S8' changed in GCC 9.1} "" { target *-*-* } 117 } g8_stdarg */

/* No change in parameter passing in GCC 9.1 for lines 120-124 (gp_stdarg
g1p_stdarg, g2p_stdarg, g4p_stdarg, g8p_stdarg), no warning expected. */
Loading

0 comments on commit 3df1a11

Please sign in to comment.