On Fri, 15 Oct 2021, Tamar Christina wrote:
> Hi All,
>
> During testing after rebasing to commit I noticed a failing testcase with the
> bitmask compare patch.
>
> Consider the following C++ testcase:
>
> #include <compare>
>
> #define A __attribute__((noipa))
> A bool f5 (double i, double j) { auto c = i <=> j; return c >= 0; }
>
> This turns into a comparison against chars, on systems where chars are signed
> the pattern inserts an unsigned convert such that it's able to do the
> transformation.
>
> i.e.:
>
> # RANGE [-1, 2]
> # c$_M_value_22 = PHI <-1(3), 0(2), 2(5), 1(4)>
> # RANGE ~[3, 254]
> _11 = (unsigned char) c$_M_value_22;
> _19 = _11 <= 1;
> # .MEM_24 = VDEF <.MEM_6(D)>
> D.10434 ={v} {CLOBBER};
> # .MEM_14 = VDEF <.MEM_24>
> D.10407 ={v} {CLOBBER};
> # VUSE <.MEM_14>
> return _19;
>
> instead of:
>
> # RANGE [-1, 2]
> # c$_M_value_5 = PHI <-1(3), 0(2), 2(5), 1(4)>
> # RANGE [-2, 2]
> _3 = c$_M_value_5 & -2;
> _19 = _3 == 0;
> # .MEM_24 = VDEF <.MEM_6(D)>
> D.10440 ={v} {CLOBBER};
> # .MEM_14 = VDEF <.MEM_24>
> D.10413 ={v} {CLOBBER};
> # VUSE <.MEM_14>
> return _19;
>
> This causes much worse codegen under -ffast-math due to phiops no longer
> recognizing the pattern. It turns out that phiopts spaceship_replacement is
> looking for the exact form that was just changed.
>
> Trying to get it to recognize the new form is not trivial as the
> transformation
> doesn't look to work when the thing it's pointing to is itself a phi-node.
What do you mean? Where it handles the BIT_AND it could also handle
the conversion, no? The later handling would probably more explicitely
need to distinguish between the BIT_AND and the conversion forms.
Jakub?
> Because of these issues this change delays the replacements until after loop
> opts. This fixes the failing C++ testcase.
>
> Bootstrapped Regtested on aarch64-none-linux-gnu,
> x86_64-pc-linux-gnu and no regressions.
>
> Ok for master?
>
> Thanks,
> Tamar
>
> gcc/ChangeLog:
>
> * match.pd: Delay bitmask compare pattern till after loop opts.
>
> --- inline copy of patch --
> diff --git a/gcc/match.pd b/gcc/match.pd
> index
> 9532cae582e152cae6e22fcce95a9744a844e3c2..d26e498447fc25a327a42cc6a119c6153d09ba03
> 100644
> --- a/gcc/match.pd
> +++ b/gcc/match.pd
> @@ -4945,7 +4945,8 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
> icmp (le le gt le gt)
> (simplify
> (cmp (bit_and:c@2 @0 cst@1) integer_zerop)
> - (with { tree csts = bitmask_inv_cst_vector_p (@1); }
> + (if (canonicalize_math_after_vectorization_p ())
> + (with { tree csts = bitmask_inv_cst_vector_p (@1); }
> (switch
> (if (csts && TYPE_UNSIGNED (TREE_TYPE (@1))
> && (VECTOR_TYPE_P (TREE_TYPE (@1)) || single_use (@2)))
> @@ -4954,7 +4955,7 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
> && (cmp == EQ_EXPR || cmp == NE_EXPR)
> && (VECTOR_TYPE_P (TREE_TYPE (@1)) || single_use (@2)))
> (with { tree utype = unsigned_type_for (TREE_TYPE (@1)); }
> - (icmp (convert:utype @0) { csts; }))))))))
> + (icmp (convert:utype @0) { csts; })))))))))
>
> /* -A CMP -B -> B CMP A. */
> (for cmp (tcc_comparison)
>
>
>
--
Richard Biener <rguent...@suse.de>
SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
Germany; GF: Felix Imendörffer; HRB 36809 (AG Nuernberg)
