Paul Koning <[email protected]> writes:
> This fixes an ICE if post-reload compare elimination is done and the
> target supports post_inc and similar modes, as pdp11 does. The ICE is
> caused by a generated PARALLEL that has the side effect twice, which
> is not legal.
>
> (Ideally it would generate a similar RTL suitable for a matching
> constraint with the side effect omitted; I may try for that later on
> if that is still supported by the constraint machinery.)
>
> Tested against my in-progress CCmode pdp11 target. Ok to commit?
Thanks for doing the conversion :-)
> Index: compare-elim.c
> ===================================================================
> --- compare-elim.c (revision 261207)
> +++ compare-elim.c (working copy)
> @@ -127,6 +127,23 @@
>
> static vec<comparison *> all_compares;
>
> +/* Callback function used by addr_side_effect_p. */
> +static int
> +addr_side_effect_check (rtx mem ATTRIBUTE_UNUSED, rtx op ATTRIBUTE_UNUSED,
> + rtx dest ATTRIBUTE_UNUSED, rtx src ATTRIBUTE_UNUSED,
> + rtx srcoff ATTRIBUTE_UNUSED, void *arg ATTRIBUTE_UNUSED)
> +{
> + return 1;
> +}
> +
> +/* Check if addr has side effects (contains autoinc or autodec
> + operations). */
> +static int
> +addr_side_effect_p (rtx addr)
> +{
> + return for_each_inc_dec (addr, addr_side_effect_check, NULL);
> +}
> +
> /* Look for a "conforming" comparison, as defined above. If valid, return
> the rtx for the COMPARE itself. */
>
> @@ -690,6 +707,13 @@
> return false;
>
> rtx src = SET_SRC (set);
> +
> + /* If the source uses addressing modes with side effects, we can't
> + do the merge because we'd end up with a PARALLEL that has two
> + instances of that side effect in it. */
> + if (addr_side_effect_p (src))
> + return false;
> +
> rtx flags = maybe_select_cc_mode (cmp, src, CONST0_RTX (GET_MODE (src)));
> if (!flags)
> {
> @@ -809,6 +833,12 @@
> else
> return false;
>
> + /* If the source uses addressing modes with side effects, we can't
> + do the merge because we'd end up with a PARALLEL that has two
> + instances of that side effect in it. */
> + if (addr_side_effect_p (cmp_src))
> + return false;
> +
> /* Determine if we ought to use a different CC_MODE here. */
> flags = maybe_select_cc_mode (cmp, cmp_src, in_b);
> if (flags == NULL)
It looks like the more general side_effects_p might be better
in both cases. It's simpler than defining a custom function,
and I'm not convinced the pass would handle other kinds of
side-effect correctly either.
FWIW, I agree these look like good places to put the check.
They're early enough to bail out, but late enough not to slow
down the common case.
Thanks,
Richard
