Alex Coplan <alex.cop...@arm.com> writes:
> We were missing validation of the candidate register operands in the
> ldp/stp pass. I was relying on recog rejecting such cases when we
> formed the final pair insn, but the testcase shows that with
> -fharden-conditionals we attempt to combine two insns with asm_operands,
> both containing mem rtxes. This then trips the assert:
>
> gcc_assert (change->new_uses.is_valid ());
>
> in the stp case as we aren't expecting to have (distinct) uses of mem in
> the candidate stores.
>
> Bootstrapped/regtested on aarch64-linux-gnu, OK for trunk?
>
> Thanks,
> Alex
>
> gcc/ChangeLog:
>
> PR target/113062
> * config/aarch64/aarch64-ldp-fusion.cc
> (ldp_bb_info::track_access): Punt on accesses with invalid
> register operands.
>
> gcc/testsuite/ChangeLog:
>
> PR target/113062
> * gcc.dg/pr113062.c: New test.
>
> diff --git a/gcc/config/aarch64/aarch64-ldp-fusion.cc
> b/gcc/config/aarch64/aarch64-ldp-fusion.cc
> index 327ba4e417d..273db8c582f 100644
> --- a/gcc/config/aarch64/aarch64-ldp-fusion.cc
> +++ b/gcc/config/aarch64/aarch64-ldp-fusion.cc
> @@ -476,6 +476,12 @@ ldp_bb_info::track_access (insn_info *insn, bool load_p,
> rtx mem)
>
> const lfs_fields lfs = { load_p, fpsimd_op_p, mem_size };
>
> + // Ignore the access if the register operand isn't suitable for ldp/stp.
> + if (!REG_P (reg_op)
> + && !SUBREG_P (reg_op)
> + && (load_p || !aarch64_const_zero_rtx_p (reg_op)))
> + return;
> +
It might be more natural to test this before:
// We want to segregate FP/SIMD accesses from GPR accesses.
//
// Before RA, we use the modes, noting that stores of constant zero
// operands use GPRs (even in non-integer modes). After RA, we use
// the hard register numbers.
const bool fpsimd_op_p
= reload_completed
? (REG_P (reg_op) && FP_REGNUM_P (REGNO (reg_op)))
: (GET_MODE_CLASS (mem_mode) != MODE_INT
&& (load_p || !aarch64_const_zero_rtx_p (reg_op)));
so that that code is running with a pre-checked operand.
Also, how about using the predicates instead:
if (load_p
? !aarch64_ldp_reg_operand (reg_op, VOIDmode)
: !aarch64_stp_reg_operand (reg_op, VOIDmode))
return;
OK with those changes, or without if you prefer.
Thanks,
Richard
> if (track_via_mem_expr (insn, mem, lfs))
> return;
>
> diff --git a/gcc/testsuite/gcc.dg/pr113062.c b/gcc/testsuite/gcc.dg/pr113062.c
> new file mode 100644
> index 00000000000..5667c17b0f6
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/pr113062.c
> @@ -0,0 +1,10 @@
> +/* { dg-do compile } */
> +/* { dg-options "-Oz -fharden-conditional-branches" } */
> +long double foo;
> +double bar;
> +void abort();
> +void check() {
> + if (foo == bar)
> + abort();
> +}
> +
