On 8/2/24 11:31 PM, Sam James wrote:
Andrew Pinski <quic_apin...@quicinc.com> writes:
Late_combine exposed this latent bug in split_live_ranges_for_shrink_wrap.
What it did was copy-prop regno 151 from regno 119 from:
```
(insn 2 264 3 2 (set (reg/f:DI 119 [ thisD.3697 ])
(reg:DI 151)) "/app/example.cpp":19:13 70 {*movdi_aarch64}
(expr_list:REG_DEAD (reg:DI 151)
(nil)))
```
into these insns:
```
(debug_insn 147 146 148 5 (var_location:DI thisD.3727 (reg/f:DI 119 [ thisD.3697 ]))
"/app/example.cpp":21:5 -1
(nil))
....
(insn 167 166 168 7 (set (reg:DI 1 x1)
(reg/f:DI 119 [ thisD.3697 ])) "/app/example.cpp":14:21 70
{*movdi_aarch64}
(nil))
```
Both are valid things to do. The problem is split_live_ranges_for_shrink_wrap
looks at the
uses of reg 151 and with and without debugging reg 151 have a different usage
in different BBs.
The function is trying to find a splitting point for reg 151 and they are
different. In the end
this causes register allocation difference.
The fix is for split_live_ranges_for_shrink_wrap to ignore uses that were in
debug insns.
Bootstrappped and tested on x86_64-linux-gnu with no regressions.
PR rtl-optimization/116179
gcc/ChangeLog:
* ira.cc (split_live_ranges_for_shrink_wrap): For the uses loop,
only look at non-debug insns.
gcc/testsuite/ChangeLog:
* g++.dg/torture/pr116179-1.C: New test.
Signed-off-by: Andrew Pinski <quic_apin...@quicinc.com>
---
gcc/ira.cc | 4 +++-
gcc/testsuite/g++.dg/torture/pr116179-1.C | 26 +++++++++++++++++++++++
2 files changed, 29 insertions(+), 1 deletion(-)
create mode 100644 gcc/testsuite/g++.dg/torture/pr116179-1.C
diff --git a/gcc/ira.cc b/gcc/ira.cc
index 5642aea3caa..5a04231f7bd 100644
--- a/gcc/ira.cc
+++ b/gcc/ira.cc
@@ -5144,7 +5144,9 @@ split_live_ranges_for_shrink_wrap (void)
use = DF_REF_NEXT_REG (use))
{
int ubbi = DF_REF_BB (use)->index;
- if (bitmap_bit_p (reachable, ubbi))
+ /* Only non debug insn should not be taken into account. */
Nit: maybe /* Only non-debug insns should be ignored. */ to remove the
double negative or /* Don't ignore non-debug insns. */.
Andrew's comment looks slightly wrong. I think he just needs to take
out the "not". "insn" should be plural as well. If he wanted to remove
the reference to "non debug" we often use "real" to refer to that case
(INSN, JUMP_INSN and CALL_INSNs).
OK with some kind of comment fix along those lines.
Jeff
