[Bug target/114732] New: ge can't be reversed to unlt for bcd compares

[Bug target/114732] New: ge can't be reversed to unlt for bcd compares

[guihaoc at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114732

            Bug ID: 114732
           Summary: ge can't be reversed to unlt for bcd compares
           Product: gcc
           Version: 14.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: target
          Assignee: unassigned at gcc dot gnu.org
          Reporter: guihaoc at gcc dot gnu.org
  Target Milestone: ---

//test.c
int
foo (vector unsigned char a, vector unsigned char b)
{
  return __builtin_vec_bcdsub_ge (a, b, 0) != 1;
}

//assembly
        bcdsub. 2,2,3,0
        cror 26,24,27
        mfcr 3,2
        rlwinm 3,3,27,1
        blr


Here ge is reversed to unlt in combine pass. 
Trying 9 -> 10:
    9: r128:SI=%6:CCFP>=0
      REG_DEAD %6:CCFP
   10: r127:SI=r128:SI^0x1
      REG_DEAD r128:SI
Successfully matched this instruction:
(set (reg:SI 127)
    (unlt:SI (reg:CCFP 106 6)
        (const_int 0 [0])))
allowing combination of insns 9 and 10
original costs 12 + 4 = 16
replacement cost 12
deferring deletion of insn with uid = 9.
modifying insn i3    10: r127:SI=unlt(%6:CCFP,0)
      REG_DEAD %6:CCFP
deferring rescan insn with uid = 10.

But it's wrong for bcd. The ge should be reversed to lt for bcd. The unorder
bit (actually it's overflow) doesn't matter. The root cause is bcd operations
use CCFP and CCFP allows reverse ge to unlt. So the bcd operations should use a
seperate CCmode.

[Bug target/114732] ge can't be reversed to unlt for bcd compares

[guihaoc at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114732

--- Comment #1 from HaoChen Gui <guihaoc at gcc dot gnu.org> ---
A straightforward test case.  It passes when compiling with O0 and aborts when
compiling with O2.

//test.c
#include <altivec.h>

#define BCD_POS0  12    //  0xC
#define BCD_NEG   13    //  0xD

void abort (void);

vector unsigned char maxbcd (unsigned int sign)
{
  vector unsigned char result;
  int i;

  for (i = 15; i > 0; i--)
    result[i] = 0x99;

  result[0] = 0x9 << 4 | sign;

  return result;
}


__attribute__ ((noinline))
int foo (vector unsigned char a, vector unsigned char b)
{
  return __builtin_vec_bcdsub_ge (a, b, 0) != 1;
}

int main(void)
{
  vector unsigned char a = maxbcd (BCD_POS0);
  vector unsigned char b = maxbcd (BCD_NEG);

  if (foo(a, b))
    abort ();

  return 0;
}

[Bug target/114732] ge can't be reversed to unlt for bcd compares

[segher at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114732

--- Comment #2 from Segher Boessenkool <segher at gcc dot gnu.org> ---
The fourth CR bit for BCD insns does not mean "unordered", it means "invalid or
overflow".  It behaves exactly as unordered though, except that it can signal
overflow as well as one of the lt gt eq conditions at the same time (so 1100,
1010, 1001 are valid bit settings from it).  This part makes CCFP not ideal,
but the codegen as shown here is correct nevertheless.

[Bug target/114732] ge can't be reversed to unlt for bcd compares

[segher at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114732

--- Comment #3 from Segher Boessenkool <segher at gcc dot gnu.org> ---
1001, 0101, 0011 I mean of course.

In some ways CCmode models this better than CCFPmode, but we do not actually
model
the SO bit (bit 3) at all in CCmode.  It is a nice feature of CCmode (that we
actually use as fundamental, in the backend code) that CCmode always has
exactly
one of three bits "hot" (and CCFPmode always one of four).  Bit 3 (SO) in
CCmode
is treated as not being part of the CC really, but an extra thing.  This
doesn't
work all that well of course.

So we really need st least three CC modes:

-- Exactly one of bits 0..3 hot, like CCFPmode;
-- Exactly one of bits 0..2 hot, bit 3 independently set, like CCmode (and
   that independent bit 3 modeled nicely as well, unlike what we have), and
   also like in the BCD insns;
-- Bit 0 is all-true, bit 2 is all-false, like in the vcmp* insns.

Do we need some other CC mode as well?  Doe we want separately named CC modes
for the different variants of this (like the integer CC mode vs. the BCD one)?
We already have a separate CCUNSmode which is exactly like CCmode, as far as
the hardware cares, but the meaning is different (for CCUNS the LT and GT bits
are set based on an unsigned integer compare, not a signed one).  There also
is CCEQmode, which has only bit 2 valid (we use it for constructing one CR bit
from others, like with cror or crnot).

[Bug target/114732] ge can't be reversed to unlt for bcd compares

[segher at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114732

--- Comment #4 from Segher Boessenkool <segher at gcc dot gnu.org> ---
(In reply to Segher Boessenkool from comment #3)
> -- Bit 0 is all-true, bit 2 is all-false, like in the vcmp* insns.

(And bits 1 and 3 are set to zeroes for those insns.  So it is all one-hot
there
as well.  But the meaning is different.)

[Bug target/114732] ge can't be reversed to unlt for bcd compares

[segher at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114732

--- Comment #5 from Segher Boessenkool <segher at gcc dot gnu.org> ---
(Or, at-most-one-hot, that is!)

[Bug target/114732] ge can't be reversed to unlt for bcd compares

[guihaoc at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114732

--- Comment #6 from HaoChen Gui <guihaoc at gcc dot gnu.org> ---
(In reply to Segher Boessenkool from comment #3)
> 1001, 0101, 0011 I mean of course.
> 
> In some ways CCmode models this better than CCFPmode, but we do not actually
> model
> the SO bit (bit 3) at all in CCmode.  It is a nice feature of CCmode (that we
> actually use as fundamental, in the backend code) that CCmode always has
> exactly
> one of three bits "hot" (and CCFPmode always one of four).  Bit 3 (SO) in
> CCmode
> is treated as not being part of the CC really, but an extra thing.  This
> doesn't
> work all that well of course.
> 
> So we really need st least three CC modes:
> 
> -- Exactly one of bits 0..3 hot, like CCFPmode;
> -- Exactly one of bits 0..2 hot, bit 3 independently set, like CCmode (and
>    that independent bit 3 modeled nicely as well, unlike what we have), and
>    also like in the BCD insns;
> -- Bit 0 is all-true, bit 2 is all-false, like in the vcmp* insns.
> 
> Do we need some other CC mode as well?  Doe we want separately named CC modes
> for the different variants of this (like the integer CC mode vs. the BCD
> one)?
> We already have a separate CCUNSmode which is exactly like CCmode, as far as
> the hardware cares, but the meaning is different (for CCUNS the LT and GT
> bits
> are set based on an unsigned integer compare, not a signed one).  There also
> is CCEQmode, which has only bit 2 valid (we use it for constructing one CR
> bit
> from others, like with cror or crnot).

Thanks for your comment. We also have VSX scalar test data class (xststdc*
insns) which use bit 0 as sign and bit 2 as matched. I think they can share
with vcmp* insns.

Could you advice if rtl code "unorder" is suitable for bcd overflow and invalid
bit test? Or we need to create UNSPECs for overflow and invalid bit test.
Thanks.