[Bug target/95083] New: x86 fp_movcc expansion depends on real_cst sharing

[Bug target/95083] New: x86 fp_movcc expansion depends on real_cst sharing

[rguenth at gcc dot gnu.org]

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

            Bug ID: 95083
           Summary: x86 fp_movcc expansion depends on real_cst sharing
           Product: gcc
           Version: 10.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: target
          Assignee: unassigned at gcc dot gnu.org
          Reporter: rguenth at gcc dot gnu.org
  Target Milestone: ---

I see gcc.target/i386/avxfp-1.c FAILing, which is

double x;
void
t()
{
  x=x>5?x:5;
}

double x;
void
q()
{
  x=x<5?x:5;
}

and q() recognized as FP min by ix86_expand_fp_movcc because the doesn't
pass prepare_cmp_insn () and later ifcvt matches up the originally
distinct pseudos for the two mentions of '5'.  For t() prepare_cmp_insn ()
succeeeds and ix86_expand_fp_movcc expands this to a UNSPEC_BLEND
(because the two mentions of '5' get a different pseudo so this doesn't
look like a max).  The first prepare_cmp_insn fails because it is fed

(lt (reg:DF 82 [ x.3_1 ])
    (const_double:DF 5.0e+0 [0x0.ap+3]))

and appearantly we cannot do a lt compare(?) (but later during ifcvt we can).

Note the above is when expanding from a COND_EXPR, thus

t ()
{
  double x.1_1;
  double iftmp.0_3;

;;   basic block 2, loop depth 0
;;    pred:       ENTRY
  x.1_1 = x;
  iftmp.0_3 = x.1_1 > 5.0e+0 ? x.1_1 : 5.0e+0;
  x = iftmp.0_3;
  return;

and

q ()
{
  double x.3_1;
  double iftmp.2_3;

;;   basic block 2, loop depth 0
;;    pred:       ENTRY
  x.3_1 = x;
  iftmp.2_3 = x.3_1 < 5.0e+0 ? x.3_1 : 5.0e+0;
  x = iftmp.2_3;
  return;

similar FAILs occur for

FAIL: gcc.target/i386/avxfp-1.c scan-assembler vmaxsd
FAIL: gcc.target/i386/avxfp-2.c scan-assembler vminsd
FAIL: gcc.target/i386/ssefp-1.c scan-assembler maxsd
FAIL: gcc.target/i386/ssefp-2.c scan-assembler minsd

So what's missing is simplification of 

Trying 8 -> 9:
    8: r87:DF=r85:DF<r82:DF
    9: r84:DF=unspec[r85:DF,r82:DF,r87:DF] 105
      REG_DEAD r87:DF
      REG_DEAD r85:DF
      REG_DEAD r82:DF
Failed to match this instruction:
(set (reg:DF 84)
    (unspec:DF [
            (reg:DF 85)
            (reg:DF 82 [ x.1_1 ])
            (lt:DF (reg:DF 85)
                (reg:DF 82 [ x.1_1 ]))
        ] UNSPEC_BLENDV))

to UNSPEC_MIN/MAX I guess?

[Bug target/95083] x86 fp_movcc expansion depends on real_cst sharing

[rguenth at gcc dot gnu.org]

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

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Version|10.0                        |11.0
           Keywords|                            |missed-optimization
                 CC|                            |uros at gcc dot gnu.org
             Target|                            |x86_64-*-* i?86-*-*

--- Comment #1 from Richard Biener <rguenth at gcc dot gnu.org> ---
Needs https://gcc.gnu.org/pipermail/gcc-patches/2020-May/545588.html to
reproduce.

[Bug target/95083] x86 fp_movcc expansion depends on real_cst sharing

[ubizjak at gmail dot com]

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

--- Comment #2 from Uroš Bizjak <ubizjak at gmail dot com> ---
It looks to me that a couple of (scalar) splitters are missing in sse.md.

There is vector

(define_insn_and_split "*<sse4_1>_blendv<ssemodesuffix><avxsizesuffix>_lt"

Defined as:

  [(set (match_operand:VF_128_256 0 "register_operand" "=Yr,*x,x")
        (unspec:VF_128_256
          [(match_operand:VF_128_256 1 "register_operand" "0,0,x")
           (match_operand:VF_128_256 2 "vector_operand" "YrBm,*xBm,xm")
           (lt:VF_128_256
             (match_operand:<sseintvecmode> 3 "register_operand" "Yz,Yz,x")
             (match_operand:<sseintvecmode> 4 "const0_operand" "C,C,C"))]
          UNSPEC_BLENDV))]

(please note const0 operand 4).

Probably similar pattern is missing that would degrade to MIN/MAX, for vector
and scalar versions.

[Bug target/95083] x86 fp_movcc expansion depends on real_cst sharing

[rguenth at gcc dot gnu.org]

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

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|---                         |FIXED
             Status|UNCONFIRMED                 |RESOLVED

--- Comment #3 from Richard Biener <rguenth at gcc dot gnu.org> ---
Fixed with the fix for PR61747.  Since min/max requires equal constants and
they are now pushed to a common reg I'm not sure we need any such new patterns.
 Maybe with integer min/max.