On Wed, Oct 15, 2025 at 10:18 AM Roger Sayle <[email protected]> wrote:
>
>
> This patch resolves PR rtl-optimization/122266 by changing the types
> of the last_set_sign_bit_copies and sign_bit_copies fields in combine.cc's
> reg_stat_type struct to be "unsigned short".  This makes both types
> consistent, and fixes the issue that on platforms where char is by
> default signed, combine.cc can overflow when handling TImode values,
> where sign_bit_copies can be 128 bits.
>
> Conveniently, there are holes (caused by field alignment/padding) in the
> reg_stat_type struct that allows us to upgrade to "unsigned short" without
> increasing the total size of the struct.  This should help reduce problems
> in future handling OImode or XImode values, or possible issues with 256-bit
> and 512-bit vector modes.  Note that it's important to take care when
> reordering the fields of this struct, as the (partial) ordering of fields
> is significant: See the use of offsetof in combine.cc's init_reg_last.
>
>
> Before:
> (gdb) ptype /o reg_stat_type
> /* offset      |    size */  type = struct reg_stat_type {
> /*      0      |       8 */    rtx_insn *last_death;
> /*      8      |       8 */    rtx_insn *last_set;
> /*     16      |       8 */    rtx last_set_value;
> /*     24      |       4 */    int last_set_table_tick;
> /*     28      |       4 */    int last_set_label;
> /*     32      |       8 */    unsigned long last_set_nonzero_bits;
> /*     40      |       1 */    char last_set_sign_bit_copies;
> /*     41: 0   |       4 */    machine_mode last_set_mode : 16;
> /*     43      |       1 */    bool last_set_invalid;
> /*     44      |       1 */    unsigned char sign_bit_copies;
> /* XXX  3-byte hole      */
> /*     48      |       8 */    unsigned long nonzero_bits;
> /*     56      |       4 */    int truncation_label;
> /*     60: 0   |       4 */    machine_mode truncated_to_mode : 16;
> /* XXX  2-byte padding   */
>                                /* total size (bytes):   64 */
>                              }
>
> After:
> /* offset      |    size */  type = struct reg_stat_type {
> /*      0      |       8 */    rtx_insn *last_death;
> /*      8      |       8 */    rtx_insn *last_set;
> /*     16      |       8 */    rtx last_set_value;
> /*     24      |       4 */    int last_set_table_tick;
> /*     28      |       4 */    int last_set_label;
> /*     32      |       8 */    unsigned long last_set_nonzero_bits;
> /*     40      |       2 */    unsigned short last_set_sign_bit_copies;
> /*     42: 0   |       4 */    machine_mode last_set_mode : 16;
> /*     44      |       1 */    bool last_set_invalid;
> /* XXX  1-byte hole      */
> /*     46      |       2 */    unsigned short sign_bit_copies;
> /*     48      |       8 */    unsigned long nonzero_bits;
> /*     56      |       4 */    int truncation_label;
> /*     60: 0   |       4 */    machine_mode truncated_to_mode : 16;
> /* XXX  2-byte padding   */
>                                /* total size (bytes):   64 */
>                              }
>
> This patch has been tested on x86_64-pc-linux-gnu with make bootstrap
> and make -k check with no new failures.  Ok for mainline?

LGTM.  Please leave Jakub time to comment though.

I'll note that for "full" TImode or larger mode support the currently
unsigned HOST_WIDE_INT last_set_nonzero_bits and nonzero_bits
fields would also need to be enlarged.  Not sure how well using
of sign_bit_copies but not nonzero_bits is guarded within combine.

Richard.

>
> 2025-10-15  Roger Sayle  <[email protected]>
>
> gcc/ChangeLog
>         PR rtl-optimization/122266
>         * combine.cc (struct reg_stat_type): Change types of sign_bit_copies
>         and last_set_sign_bit_copies to unsigned short, to avoid overflows
>         on TImode (and wider) values.
>
> gcc/testsuite/ChangeLog
>         * gcc.target/i386/pr122266.c: New test case.
>
>
> Thanks in advance,
> Roger
> --
>

Reply via email to