Vladimir Makarov via Gcc-patches <gcc-patches@gcc.gnu.org> writes:
> The following fixes
>
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97978
>
> The patch was successfully bootstrapped on x86-64.
Can you explain this a bit more? The assert fires if the register
allocation is inconsistent with the conflict information. What causes
the inconsistency in this case, and why is it OK for the inconsistency
to persist until the next lra_assign pass? Does something fix up the
inconsistency later, or is the inconsistent information never used?
Is there no chance of lra_split_hard_reg_for updating the information
itself, to keep everything self-consistent? Bypassing the check for
every pseudo register seems like quite a big hammer.
I'm not saying this is the wrong fix. I just think it would help
to have more commentary explaining the situation.
Thanks,
Richard
>
>
> commit fbf9b2b634e376516cd21d7aa92ef3fd5778aa10 (HEAD -> master)
> Author: Vladimir N. Makarov <vmaka...@redhat.com>
> Date: Wed Jan 6 14:48:53 2021 -0500
>
> [PR97978] LRA: Permit temporary allocation incorrectness after hard reg
> split.
>
> LRA can crash when a hard register was split and the same hard register
> was assigned on the previous assignment sub-pass. The following
> patch fixes this problem.
>
> gcc/ChangeLog:
>
> PR rtl-optimization/97978
> * lra-int.h (lra_hard_reg_split_p): New external.
> * lra.c (lra_hard_reg_split_p): New global.
> (lra): Set up lra_hard_reg_split_p after splitting a hard reg.
> * lra-assigns.c (lra_assign): Don't check allocation correctness
> after hard reg splitting.
>
> gcc/testsuite/ChangeLog:
>
> PR rtl-optimization/97978
> * gcc.target/i386/pr97978.c: New.
>
> diff --git a/gcc/lra-assigns.c b/gcc/lra-assigns.c
> index 9335e4c876e..c6a941fe663 100644
> --- a/gcc/lra-assigns.c
> +++ b/gcc/lra-assigns.c
> @@ -1636,10 +1636,11 @@ lra_assign (bool &fails_p)
> bitmap_initialize (&all_spilled_pseudos, ®_obstack);
> create_live_range_start_chains ();
> setup_live_pseudos_and_spill_after_risky_transforms (&all_spilled_pseudos);
> - if (! lra_asm_error_p && flag_checking)
> - /* Check correctness of allocation for call-crossed pseudos but
> - only when there are no asm errors as in the case of errors the
> - asm is removed and it can result in incorrect allocation. */
> + if (! lra_hard_reg_split_p && ! lra_asm_error_p && flag_checking)
> + /* Check correctness of allocation but only when there are no hard reg
> + splits and asm errors as in the case of errors explicit insns
> involving
> + hard regs are added or the asm is removed and this can result in
> + incorrect allocation. */
> for (i = FIRST_PSEUDO_REGISTER; i < max_regno; i++)
> if (lra_reg_info[i].nrefs != 0
> && reg_renumber[i] >= 0
> diff --git a/gcc/lra-int.h b/gcc/lra-int.h
> index 75ba6560bcc..1b8f7b6ae61 100644
> --- a/gcc/lra-int.h
> +++ b/gcc/lra-int.h
> @@ -273,6 +273,7 @@ typedef class lra_insn_recog_data *lra_insn_recog_data_t;
>
> extern FILE *lra_dump_file;
>
> +extern bool lra_hard_reg_split_p;
> extern bool lra_asm_error_p;
> extern bool lra_reg_spill_p;
>
> diff --git a/gcc/lra.c b/gcc/lra.c
> index 380a21ac2ac..aa49de6f154 100644
> --- a/gcc/lra.c
> +++ b/gcc/lra.c
> @@ -2211,6 +2211,9 @@ bitmap_head lra_subreg_reload_pseudos;
> /* File used for output of LRA debug information. */
> FILE *lra_dump_file;
>
> +/* True if we split hard reg after the last constraint sub-pass. */
> +bool lra_hard_reg_split_p;
> +
> /* True if we found an asm error. */
> bool lra_asm_error_p;
>
> @@ -2359,6 +2362,7 @@ lra (FILE *f)
> if (live_p)
> lra_clear_live_ranges ();
> bool fails_p;
> + lra_hard_reg_split_p = false;
> do
> {
> /* We need live ranges for lra_assign -- so build them.
> @@ -2403,6 +2407,7 @@ lra (FILE *f)
> live_p = false;
> if (! lra_split_hard_reg_for ())
> break;
> + lra_hard_reg_split_p = true;
> }
> }
> while (fails_p);
> diff --git a/gcc/testsuite/gcc.target/i386/pr97978.c
> b/gcc/testsuite/gcc.target/i386/pr97978.c
> new file mode 100644
> index 00000000000..263bca8708d
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/i386/pr97978.c
> @@ -0,0 +1,22 @@
> +/* { dg-do compile } */
> +/* { dg-options "-Os -fno-PIC" } */
> +int sg;
> +long int kk;
> +
> +void
> +bp (int jz, int tj, long int li)
> +{
> + if (jz == 0 || tj == 0)
> + __builtin_unreachable ();
> +
> + kk = li;
> +}
> +
> +void
> +qp (void)
> +{
> + ++kk;
> +
> + for (;;)
> + bp (1l / sg, 0, ~0u);
> +}
