diff options
author | Jakub Jelinek <jakub@redhat.com> | 2025-04-12 13:13:53 +0200 |
---|---|---|
committer | Jakub Jelinek <jakub@gcc.gnu.org> | 2025-04-12 13:13:53 +0200 |
commit | 3f9dfb94eab1ab1bbf9a2b5e20d1f61e36516063 (patch) | |
tree | 49ae959b0458af60123da06cf4e63ab13dcae058 | |
parent | 7e91bba6d53899689b00bd0c995b35f6586fcacd (diff) | |
download | gcc-3f9dfb94eab1ab1bbf9a2b5e20d1f61e36516063.zip gcc-3f9dfb94eab1ab1bbf9a2b5e20d1f61e36516063.tar.gz gcc-3f9dfb94eab1ab1bbf9a2b5e20d1f61e36516063.tar.bz2 |
bitintlower: Fix up handling of SSA_NAME copies in coalescing [PR119722]
The following patch is miscompiled, because during the limited
SSA name coalescing the bitintlower pass does we incorrectly don't
register a conflict.
This is on
<bb 4> [local count: 1073741824]:
# b_17 = PHI <b_19(3), 8(2)>
g.4_13 = g;
_14 = g.4_13 >> 50;
_15 = (unsigned int) _14;
_21 = b_17;
_16 = (unsigned int) _21;
s_22 = _15 + _16;
return s_22;
basic block where in the map->bitint bitmap we track 14, 17 and 19.
The build_bitint_stmt_ssa_conflicts "hook" has special code where
it tracks uses at the final statements of mergeable operations, so
e.g. the
_16 = (unsigned int) _21;
statement is considered to be use of b_17 because _21 is not in
map->bitmap (or large_huge.m_names), i.e. is mergeable.
The problem is that build_ssa_conflict_graph has special code to handle
SSA_NAME copies and _21 = b_17; is gimple_assign_copy_p. In such cases
it calls live_track_clear_var on the rhs1. The problem is that
on the above bb, after we note in the _16 = (unsigned int) _21;
stmt we need b_17 the generic code makes us forget that because
of the copy statement, and then build_bitint_stmt_ssa_conflicts
ignores it completely (because _21 is large/huge bitint and is
not in map->bitint, so assumed to be handled by a later stmt in the
bb, for backwards walk like this before this one).
As the b_17 use is ignored, the coalescing thinks it can put
all of b_17, b_19 and _14 into the same partition, which is wrong,
while we can and should coalesce b_17 and b_19, _14 needs to be a different
temporary because b_17 is set before and used after _14 has been written.
The following patch fixes it by handling gimple_assign_copy_p in two
separate spots, move the generic coalesce handling of it after
build_ssa_conflict_graph (where build_ssa_conflict_graph handling
doesn't fall through to that, it does continue after the call) and
inside of build_ssa_conflict_graph it performs it too, but only if
the lhs is not mergeable large/huge bitint.
2025-04-12 Jakub Jelinek <jakub@redhat.com>
PR tree-optimization/119722
* gimple-lower-bitint.h (build_bitint_stmt_ssa_conflicts): Add
CLEAR argument.
* gimple-lower-bitint.cc (build_bitint_stmt_ssa_conflicts): Add
CLEAR argument. Call clear on gimple_assign_copy_p rhs1 if lhs
is large/huge bitint unless lhs is not in names.
* tree-ssa-coalesce.cc (build_ssa_conflict_graph): Adjust
build_bitint_stmt_ssa_conflicts caller. Move gimple_assign_copy_p
handling to after the build_bitint_stmt_ssa_conflicts call.
* gcc.dg/torture/bitint-77.c: New test.
-rw-r--r-- | gcc/gimple-lower-bitint.cc | 22 | ||||
-rw-r--r-- | gcc/gimple-lower-bitint.h | 1 | ||||
-rw-r--r-- | gcc/testsuite/gcc.dg/torture/bitint-77.c | 26 | ||||
-rw-r--r-- | gcc/tree-ssa-coalesce.cc | 22 |
4 files changed, 60 insertions, 11 deletions
diff --git a/gcc/gimple-lower-bitint.cc b/gcc/gimple-lower-bitint.cc index 3425816..c52a657 100644 --- a/gcc/gimple-lower-bitint.cc +++ b/gcc/gimple-lower-bitint.cc @@ -5919,7 +5919,8 @@ build_bitint_stmt_ssa_conflicts (gimple *stmt, live_track *live, ssa_conflicts *graph, bitmap names, void (*def) (live_track *, tree, ssa_conflicts *), - void (*use) (live_track *, tree)) + void (*use) (live_track *, tree), + void (*clear) (live_track *, tree)) { bool muldiv_p = false; tree lhs = NULL_TREE; @@ -5936,6 +5937,25 @@ build_bitint_stmt_ssa_conflicts (gimple *stmt, live_track *live, { if (!bitmap_bit_p (names, SSA_NAME_VERSION (lhs))) return; + + /* A copy between 2 partitions does not introduce an interference + by itself. If they did, you would never be able to coalesce + two things which are copied. If the two variables really do + conflict, they will conflict elsewhere in the program. + + This is handled by simply removing the SRC of the copy from + the live list, and processing the stmt normally. + + Don't do this if lhs is not in names though, in such cases + it is actually used at some point later in the basic + block. */ + if (gimple_assign_copy_p (stmt)) + { + tree rhs1 = gimple_assign_rhs1 (stmt); + if (TREE_CODE (rhs1) == SSA_NAME) + clear (live, rhs1); + } + switch (gimple_assign_rhs_code (stmt)) { case MULT_EXPR: diff --git a/gcc/gimple-lower-bitint.h b/gcc/gimple-lower-bitint.h index 8662c4b..4798484 100644 --- a/gcc/gimple-lower-bitint.h +++ b/gcc/gimple-lower-bitint.h @@ -26,6 +26,7 @@ extern void build_bitint_stmt_ssa_conflicts (gimple *, live_track *, ssa_conflicts *, bitmap, void (*) (live_track *, tree, ssa_conflicts *), + void (*) (live_track *, tree), void (*) (live_track *, tree)); #endif /* GCC_GIMPLE_LOWER_BITINT_H */ diff --git a/gcc/testsuite/gcc.dg/torture/bitint-77.c b/gcc/testsuite/gcc.dg/torture/bitint-77.c new file mode 100644 index 0000000..3e2523f --- /dev/null +++ b/gcc/testsuite/gcc.dg/torture/bitint-77.c @@ -0,0 +1,26 @@ +/* PR tree-optimization/119722 */ +/* { dg-do run { target bitint } } */ +/* { dg-options "-O2 -fno-tree-forwprop -fno-tree-copy-prop -fno-tree-fre" } */ + +#if __BITINT_MAXWIDTH__ >= 33300 +unsigned _BitInt(33300) g; + +unsigned +foo (long c) +{ + unsigned _BitInt(33300) b + = __builtin_stdc_rotate_left ((unsigned _BitInt(13)) 8, c); + return ((unsigned _BitInt(50)) (g >> 50) + + ({ unsigned _BitInt(300) unused; b; })); +} +#endif + +int +main () +{ +#if __BITINT_MAXWIDTH__ >= 33300 + unsigned x = foo (0); + if (x != 8) + __builtin_abort (); +#endif +} diff --git a/gcc/tree-ssa-coalesce.cc b/gcc/tree-ssa-coalesce.cc index b78ffd7..7cc39f7 100644 --- a/gcc/tree-ssa-coalesce.cc +++ b/gcc/tree-ssa-coalesce.cc @@ -896,6 +896,18 @@ build_ssa_conflict_graph (tree_live_info_p liveinfo) tree var; gimple *stmt = gsi_stmt (gsi); + if (is_gimple_debug (stmt)) + continue; + + if (map->bitint) + { + build_bitint_stmt_ssa_conflicts (stmt, live, graph, map->bitint, + live_track_process_def, + live_track_process_use, + live_track_clear_var); + continue; + } + /* A copy between 2 partitions does not introduce an interference by itself. If they did, you would never be able to coalesce two things which are copied. If the two variables really do @@ -912,16 +924,6 @@ build_ssa_conflict_graph (tree_live_info_p liveinfo) && TREE_CODE (rhs1) == SSA_NAME) live_track_clear_var (live, rhs1); } - else if (is_gimple_debug (stmt)) - continue; - - if (map->bitint) - { - build_bitint_stmt_ssa_conflicts (stmt, live, graph, map->bitint, - live_track_process_def, - live_track_process_use); - continue; - } /* For stmts with more than one SSA_NAME definition pretend all the SSA_NAME outputs but the first one are live at this point, so |