On Tue, Sep 29, 2026 at 10:21 AM Dominic P <[email protected]> wrote:
>
> After three uses find_flags_uses_in_insn gives up and marks the uses
> of a compare as unknown, so try_eliminate_compare keeps the original
> CC mode and fails when the arithmetic only sets a narrower one, as
> ANDS, ADDS and SUBS do on Arm and AArch64. Four conditional moves or
> csels fed by one "and; cmp #0" keep the compare. Record the uses in
> an auto_vec with inline room for three, so the common case still
> needs no heap allocation.
>
> Arm -O2 Linux .text: bcmrpi_defconfig -68 B (13 functions shrink, none
> grow), bcm2709_defconfig -44 B. No gcc.c-torture asm change on x86_64
> or AArch64.
>
> Bootstrapped and regression tested on x86_64-pc-linux-gnu; regression
> tested on arm-none-eabi and aarch64-linux-gnu.
>
> Assisted-by: Claude Opus 5 (Anthropic)
>
> gcc/ChangeLog:
>
> * compare-elim.cc (MAX_CMP_USE): Remove.
> (struct comparison): Make uses an auto_vec. Remove n_uses.
> (find_flags_uses_in_insn): Do not limit the number of uses.
> (maybe_select_cc_mode): Use uses.length ().
> (find_comparison_dom_walker::before_dom_children): Allocate
> comparisons with new.
> (execute_compare_elim_after_reload): Free them with delete.
>
> gcc/testsuite/ChangeLog:
>
> * gcc.target/aarch64/cmpelim_mult_uses_2.c: New test.
> * gcc.target/arm/cmpelim_mult_uses_1.c: New test.
>
> Signed-off-by: Dominic P <[email protected]>
> ---
> Changes in v2, from Andrea Pinski's review:
> - uses is an auto_vec<comparison_use, 3>, so up to three uses need no
> allocation, as before; hence new/delete in place of XCNEW.
> - The push names comparison_use.
>
> Uses seen: at most 7 on one compare (none eliminated); each
> Arm kernel has 11 eliminations needing more than 3 uses, 4 of them 5
> (e.g. fbcon_resize), and nothing needed 6. So 5 would cover all I saw
> (two Arm kernels, two Arm firmwares, gcc.c-torture on Arm, AArch64 and
> x86_64, ten large GCC sources), but unlimited costs nothing: 158 of
> bcmrpi's 187,237 compares need the heap, and cc1plus time on
> insn-recog-3.cc is unchanged.
>
> x86_64 bootstrap and regression test redone; the Arm and AArch64 runs
> are v1's, as v2 generates identical gcc.c-torture asm on all three.
>
> OK for trunk?
>
> gcc/compare-elim.cc | 30 ++++------------
> .../gcc.target/aarch64/cmpelim_mult_uses_2.c | 36 +++++++++++++++++++
> .../gcc.target/arm/cmpelim_mult_uses_1.c | 31 ++++++++++++++++
> 3 files changed, 73 insertions(+), 24 deletions(-)
> create mode 100644 gcc/testsuite/gcc.target/aarch64/cmpelim_mult_uses_2.c
> create mode 100644 gcc/testsuite/gcc.target/arm/cmpelim_mult_uses_1.c
>
> diff --git a/gcc/compare-elim.cc b/gcc/compare-elim.cc
> index 6218324e1ab..560b7ee7919 100644
> --- a/gcc/compare-elim.cc
> +++ b/gcc/compare-elim.cc
> @@ -71,12 +71,6 @@ along with GCC; see the file COPYING3. If not see
>
> /* These structures describe a comparison and how it is used. */
>
> -/* The choice of maximum 3 uses comes from wanting to eliminate the two
> - duplicate compares from a three-way branch on the sign of a value.
> - This is also sufficient to eliminate the duplicate compare against the
> - high-part of a double-word comparison. */
> -#define MAX_CMP_USE 3
> -
> struct comparison_use
> {
> /* The instruction in which the result of the compare is used. */
> @@ -106,17 +100,12 @@ struct comparison
> rtx eh_note;
>
> /* Information about how this comparison is used. */
> - struct comparison_use uses[MAX_CMP_USE];
> + auto_vec<comparison_use, 3> uses;
>
> /* The original CC_MODE for this comparison. */
> machine_mode orig_mode;
>
> - /* The number of uses identified for this comparison. */
> - unsigned short n_uses;
> -
> - /* True if not all uses of this comparison have been identified.
> - This can happen either for overflowing the array above, or if
> - the flags register is used in some unusual context. */
> + /* True if not all uses of this comparison have been identified. */
> bool missing_uses;
>
> /* True if its inputs are still valid at the end of the block. */
> @@ -243,10 +232,6 @@ find_flags_uses_in_insn (struct comparison *cmp,
> rtx_insn *insn)
> if (DF_REF_TYPE (use) != DF_REF_REG_USE)
> goto fail;
>
> - /* If we've run out of slots to record uses, quit. */
> - if (cmp->n_uses == MAX_CMP_USE)
> - goto fail;
> -
> /* Unfortunately the location of the flags register, while present
> in the reference structure, doesn't help. We need to find the
> comparison code that is outer to the actual flags use. */
> @@ -265,10 +250,7 @@ find_flags_uses_in_insn (struct comparison *cmp,
> rtx_insn *insn)
> && XEXP (x, 1) == const0_rtx)
> {
> /* We've found a use of the flags that we understand. */
> - struct comparison_use *cuse = &cmp->uses[cmp->n_uses++];
> - cuse->insn = insn;
> - cuse->loc = loc;
> - cuse->code = GET_CODE (x);
> + cmp->uses.safe_push (comparison_use { insn, loc, GET_CODE (x) });
> }
> else
> goto fail;
> @@ -388,7 +370,7 @@ find_comparison_dom_walker::before_dom_children
> (basic_block bb)
> continue;
> }
>
> - last_cmp = XCNEW (struct comparison);
> + last_cmp = new comparison ();
> last_cmp->insn = insn;
> last_cmp->prev_clobber = last_clobber;
> last_cmp->in_a = strip_not (XEXP (src, 0));
> @@ -508,7 +490,7 @@ maybe_select_cc_mode (struct comparison *cmp, rtx a
> ATTRIBUTE_UNUSED,
> rtx b ATTRIBUTE_UNUSED)
> {
> machine_mode sel_mode;
> - const int n = cmp->n_uses;
> + const int n = cmp->uses.length ();
> rtx flags = NULL;
>
> #ifndef SELECT_CC_MODE
> @@ -925,7 +907,7 @@ execute_compare_elim_after_reload (void)
> FOR_EACH_VEC_ELT (all_compares, i, cmp)
> {
> try_eliminate_compare (cmp);
> - XDELETE (cmp);
> + delete cmp;
> }
Hmm, can you use pop instead? Or does this need to be done in a specific order?
If not then can you convert the loop to a pop instead.
Otherwise it would be better to change the loop to:
for (auto &cmp : all_compares)
{
try_eliminate_compare (cmp);
delete cmp;
cmp = nullptr;
}
Just to make sure the element is not used again.
I am trying to stop `an use after free` usage from happening if the
code is changed afterwards.
>
> all_compares.release ();
> diff --git a/gcc/testsuite/gcc.target/aarch64/cmpelim_mult_uses_2.c
> b/gcc/testsuite/gcc.target/aarch64/cmpelim_mult_uses_2.c
> new file mode 100644
> index 00000000000..3153956cdd3
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/aarch64/cmpelim_mult_uses_2.c
> @@ -0,0 +1,36 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2" } */
> +/* { dg-final { check-function-bodies "**" "" "" } } */
> +
> +/*
> +** f:
> +** ands w0, w0, w1
> +** ...
> +*/
> +int
> +f (int x, int y, int a, int b, int c, int d, int *p)
> +{
> + int t = x & y;
> + p[0] = t ? a : b;
> + p[1] = t ? b : c;
> + p[2] = t ? c : d;
> + p[3] = t ? d : a;
> + return t;
> +}
> +
> +/*
> +** g:
> +** subs x0, x0, x1
> +** ...
> +*/
> +long long
> +g (long long x, long long y, long long a, long long b, long long c,
> + long long d, long long *p)
> +{
> + long long t = x - y;
> + p[0] = t ? a : b;
> + p[1] = t ? b : c;
> + p[2] = t ? c : d;
> + p[3] = t ? d : a;
> + return t;
> +}
> diff --git a/gcc/testsuite/gcc.target/arm/cmpelim_mult_uses_1.c
> b/gcc/testsuite/gcc.target/arm/cmpelim_mult_uses_1.c
> new file mode 100644
> index 00000000000..b64914d5389
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/arm/cmpelim_mult_uses_1.c
> @@ -0,0 +1,31 @@
> +/* { dg-do compile } */
> +/* { dg-require-effective-target arm32 } */
> +/* { dg-options "-O2" } */
> +/* { dg-final { check-function-bodies "**" "" "" } } */
> +
> +/*
> +** f:
> +** ands r0, r0, #2097152
> +** ...
> +*/
> +void
> +f (unsigned x, int *p, int *q)
> +{
> + int t = x & (1 << 21);
> + *p = t ? 3 : 1;
> + *q = t ? 2 : 0;
> +}
> +
> +/*
> +** g:
> +** adds r0, r0, r1
> +** ...
> +*/
> +int
> +g (int x, int y, int *p, int *q)
> +{
> + int t = x + y;
> + *p = t ? 3 : 1;
> + *q = t ? 5 : 0;
> + return t;
> +}
> --
> 2.55.0
>
