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.  */.

(Not sure if I've confused myself with the negatives there)

> +       if (NONDEBUG_INSN_P (DF_REF_INSN (use))
> +           && bitmap_bit_p (reachable, ubbi))
>           bitmap_set_bit (need_new, ubbi);
>       }
>        last_interesting_insn = insn;
> [...]

thanks,
sam

Reply via email to