From c00e1e3aa5ae62a991d105d309061d12f6a8764f Mon Sep 17 00:00:00 2001 From: Roger Sayle Date: Tue, 7 Jun 2022 10:09:49 +0100 Subject: [PATCH] PR middle-end/105853: Call store_constructor directly from calls.cc. This patch fixes both ICE regressions PR middle-end/105853 and PR target/105856 caused by my recent patch to expand small const structs as immediate constants. That patch updated code generation in three places: two in expr.cc that call store_constructor directly, and the third in calls.cc's load_register_parameters that expands its CONSTRUCTOR via expand_expr, as store_constructor is local/static to expr.cc, and the "public" API, should usually simply forward the constructor to the appropriate store_constructor function. Alas, despite the clean regression testing on multiple targets, the above ICEs show that expand_expr isn't a suitable proxy for store_constructor, and things that (I'd assumed) shouldn't affect how/whether a struct is placed in a register [such as whether the struct is considered packed/ aligned or not] actually interfere with the optimization that is being attempted. The (proposed) solution is to export store_constructor (and it's helper function int_expr_size) from expr.cc, by removing their static qualifier and prototyping both functions in expr.h, so they can be called directly from load_register_parameters in calls.cc. This cures both ICEs, but almost as importantly improves code generation over GCC 12. For PR 105853, GCC 12 generates: compose_nd_na_ipv6_src: movzx eax, WORD PTR eth_addr_zero[rip+2] movzx edx, WORD PTR eth_addr_zero[rip] movzx edi, WORD PTR eth_addr_zero[rip+4] sal rax, 16 or rax, rdx sal rdi, 32 or rdi, rax xor eax, eax jmp packet_set_nd eth_addr_zero: .zero 6 where now (with this fix) GCC 13 generates: compose_nd_na_ipv6_src: xorl %edi, %edi xorl %eax, %eax jmp packet_set_nd Likewise, for PR 105856 on ARM, we'd previously generate: g_329_3: movw r3, #:lower16:.LANCHOR0 movt r3, #:upper16:.LANCHOR0 ldr r0, [r3] b func_19 but with this optimization we now generate: g_329_3: mov r0, #6 b func_19 2022-06-07 Roger Sayle gcc/ChangeLog PR middle-end/105853 PR target/105856 * calls.cc (load_register_parameters): Call store_constructor and int_expr_size directly instead of expanding via expand_expr. * expr.cc (static void store_constructor): Don't prototype here. (static HOST_WIDE_INT int_expr_size): Likewise. (store_constructor): No longer static. (int_expr_size): Likewise, no longer static. * expr.h (store_constructor): Prototype here. (int_expr_size): Prototype here. gcc/testsuite/ChangeLog PR middle-end/105853 PR target/105856 * gcc.dg/pr105853.c: New test case. * gcc.dg/pr105856.c: New test case. --- gcc/calls.cc | 7 ++++--- gcc/expr.cc | 6 ++---- gcc/expr.h | 2 ++ gcc/testsuite/gcc.dg/pr105853.c | 11 +++++++++++ gcc/testsuite/gcc.dg/pr105856.c | 10 ++++++++++ 5 files changed, 29 insertions(+), 7 deletions(-) create mode 100644 gcc/testsuite/gcc.dg/pr105853.c create mode 100644 gcc/testsuite/gcc.dg/pr105856.c diff --git a/gcc/calls.cc b/gcc/calls.cc index a4336c10a88..f4e1299505e 100644 --- a/gcc/calls.cc +++ b/gcc/calls.cc @@ -2186,10 +2186,11 @@ load_register_parameters (struct arg_data *args, int num_actuals, && immediate_const_ctor_p (DECL_INITIAL (tree_value))) { rtx target = gen_reg_rtx (word_mode); - rtx x = expand_expr (DECL_INITIAL (tree_value), - target, word_mode, EXPAND_NORMAL); + store_constructor (DECL_INITIAL (tree_value), target, 0, + int_expr_size (DECL_INITIAL (tree_value)), + false); reg = gen_rtx_REG (word_mode, REGNO (reg)); - emit_move_insn (reg, x); + emit_move_insn (reg, target); } else if (partial == 0 || args[i].pass_on_stack) { diff --git a/gcc/expr.cc b/gcc/expr.cc index fb062dc8475..85cb4141195 100644 --- a/gcc/expr.cc +++ b/gcc/expr.cc @@ -84,7 +84,6 @@ static void emit_block_move_via_loop (rtx, rtx, rtx, unsigned); static void clear_by_pieces (rtx, unsigned HOST_WIDE_INT, unsigned int); static rtx_insn *compress_float_constant (rtx, rtx); static rtx get_subtarget (rtx); -static void store_constructor (tree, rtx, int, poly_int64, bool); static rtx store_field (rtx, poly_int64, poly_int64, poly_uint64, poly_uint64, machine_mode, tree, alias_set_type, bool, bool); @@ -100,7 +99,6 @@ static void do_tablejump (rtx, machine_mode, rtx, rtx, rtx, profile_probability); static rtx const_vector_from_tree (tree); static tree tree_expr_size (const_tree); -static HOST_WIDE_INT int_expr_size (const_tree); static void convert_mode_scalar (rtx, rtx, int); @@ -6757,7 +6755,7 @@ fields_length (const_tree type) which has been packed to exclude padding bits. If REVERSE is true, the store is to be done in reverse order. */ -static void +void store_constructor (tree exp, rtx target, int cleared, poly_int64 size, bool reverse) { @@ -13209,7 +13207,7 @@ expr_size (tree exp) /* Return a wide integer for the size in bytes of the value of EXP, or -1 if the size can vary or is larger than an integer. */ -static HOST_WIDE_INT +HOST_WIDE_INT int_expr_size (const_tree exp) { tree size; diff --git a/gcc/expr.h b/gcc/expr.h index d777c284abe..03511832405 100644 --- a/gcc/expr.h +++ b/gcc/expr.h @@ -339,6 +339,8 @@ extern bool categorize_ctor_elements (const_tree, HOST_WIDE_INT *, HOST_WIDE_INT *, HOST_WIDE_INT *, bool *); extern bool immediate_const_ctor_p (const_tree, unsigned int words = 1); +extern void store_constructor (tree, rtx, int, poly_int64, bool); +extern HOST_WIDE_INT int_expr_size (const_tree exp); extern void expand_operands (tree, tree, rtx, rtx*, rtx*, enum expand_modifier); diff --git a/gcc/testsuite/gcc.dg/pr105853.c b/gcc/testsuite/gcc.dg/pr105853.c new file mode 100644 index 00000000000..4f234ac11d9 --- /dev/null +++ b/gcc/testsuite/gcc.dg/pr105853.c @@ -0,0 +1,11 @@ +/* { dg-do compile } */ +/* { dg-options "-O2" } */ + +struct { + struct { + short e16[3]; + } +} const eth_addr_zero = {{}}; /* { dg-warning "no semicolon at" } */ +void compose_nd_na_ipv6_src() { + packet_set_nd(eth_addr_zero); /* { dg-warning "implicit declaration" } */ +} diff --git a/gcc/testsuite/gcc.dg/pr105856.c b/gcc/testsuite/gcc.dg/pr105856.c new file mode 100644 index 00000000000..dd3aa2f18c0 --- /dev/null +++ b/gcc/testsuite/gcc.dg/pr105856.c @@ -0,0 +1,10 @@ +/* { dg-do compile } */ +/* { dg-options "-O2" } */ +#pragma pack(1) +struct { + unsigned f0; +} static g_251 = {6}; +void g_329_3() { + func_19(g_251); /* { dg-warning "implicit declaration" } */ +} +