Hi!
On Mon, May 30, 2022 at 06:12:26PM +0800, Kewen.Lin wrote:
> on 2022/5/26 15:35, HAO CHEN GUI wrote:
> > This patch fixes the ICE reported in PR100736. It removes the condition
> > check of finite math only flag not setting in "*<code><mode>_cc" pattern.
> > With or without this flag, we still can use "cror" to check if either
> > two bits of CC is set or not for "fp_two" codes. We don't need a reverse
> > comparison (implemented by crnot) here when the finite math flag is set,
> > as the latency of "cror" and "crnor" are the same.
> > --- a/gcc/config/rs6000/rs6000.md
> > +++ b/gcc/config/rs6000/rs6000.md
> > @@ -12995,9 +12995,9 @@ (define_insn_and_split "*<code><mode>_cc"
> > [(set (match_operand:GPR 0 "gpc_reg_operand" "=r")
> > (fp_two:GPR (match_operand:CCFP 1 "cc_reg_operand" "y")
> > (const_int 0)))]
> > - "!flag_finite_math_only"
> > + ""
> > "#"
> > - "&& 1"
> > + ""
>
> Segher added this hunk, not sure if he prefer to keep the condition unchanged
> and update the expansion side, looking forward to his comments. :)
It's not clear to me how this can ever happen without finite_math_only?
The patch is safe, sure, but it may the real problem is elsewhere.
> > --- /dev/null
> > +++ b/gcc/testsuite/gcc.target/powerpc/pr100736.c
> > @@ -0,0 +1,12 @@
> > +/* { dg-do compile } */
> > +/* { dg-require-effective-target powerpc_p8vector_ok } */
> > +/* { dg-options "-mdejagnu-cpu=power8 -O2 -ffinite-math-only" } */
The usual flag to use would be -ffast-math :-)
> > +/* { dg-final { scan-assembler {\mcror\M} } } */
>
> The case of PR100736 fails with ICE as reported, maybe we can remove this
> dg-final check,
> since as you noted in the description above either "cror" or "crnor" are
> acceptable,
> this extra check could probably make this case fragile.
Check for \mcrn?or\M then? But, is crnor something we want here ever?
The reason we do not have cror for finte-math-only is that comparisons
can only (validly :-) ) return LT, GT, or EQ then, and we can branch on
that without twiddling CRF bits first. Is this not true for BCD
compares, is that what the problem is? Or, is our builtin expansion
returning something invalid? Or something else :-)
Segher
