On Fri, Oct 28, 2022 at 1:56 PM Hongtao Liu <[email protected]> wrote:
>
> On Thu, Oct 27, 2022 at 2:59 AM H.J. Lu via Gcc-patches
> <[email protected]> wrote:
> >
> > In i386.md, neg patterns which set MODE_CC register like
> >
> > (set (reg:CCC FLAGS_REG)
> > (ne:CCC (match_operand:SWI48 1 "general_reg_operand") (const_int 0)))
> >
> > can lead to errors when operand 1 is a constant value. If FLAGS_REG in
> >
> > (set (reg:CCC FLAGS_REG)
> > (ne:CCC (const_int 2) (const_int 0)))
> >
> > is set to 1, RTX simplifiers may simplify
> >
> > (set (reg:SI 93)
> > (neg:SI (ltu:SI (reg:CCC 17 flags) (const_int 0 [0]))))
> >
> > as
> >
> > (set (reg:SI 93)
> > (neg:SI (ltu:SI (const_int 1) (const_int 0 [0]))))
> >
> > which leads to incorrect results since LTU on MODE_CC register isn't the
> > same as "unsigned less than" in x86 backend. To prevent RTL optimizers
> > from setting MODE_CC register to a constant, use UNSPEC_CC_NE to replace
> > ne:CCC/ne:CCO when setting FLAGS_REG in neg patterns.
> >
> > gcc/
> >
> > PR target/107172
> > * config/i386/i386.md (UNSPEC_CC_NE): New.
> > Replace ne:CCC/ne:CCO with UNSPEC_CC_NE in neg patterns.
> >
> > gcc/testsuite/
> >
> > PR target/107172
> > * gcc.target/i386/pr107172.c: New test.
> > ---
> > gcc/config/i386/i386.md | 45 +++++++++++++-----------
> > gcc/testsuite/gcc.target/i386/pr107172.c | 26 ++++++++++++++
> > 2 files changed, 51 insertions(+), 20 deletions(-)
> > create mode 100644 gcc/testsuite/gcc.target/i386/pr107172.c
> >
> > diff --git a/gcc/config/i386/i386.md b/gcc/config/i386/i386.md
> > index baf1f1f8fa2..aaa678e7314 100644
> > --- a/gcc/config/i386/i386.md
> > +++ b/gcc/config/i386/i386.md
> > @@ -113,6 +113,7 @@ (define_c_enum "unspec" [
> > UNSPEC_PEEPSIB
> > UNSPEC_INSN_FALSE_DEP
> > UNSPEC_SBB
> > + UNSPEC_CC_NE
> >
> > ;; For SSE/MMX support:
> > UNSPEC_FIX_NOTRUNC
> > @@ -11470,7 +11471,7 @@ (define_insn_and_split "*neg<dwi>2_doubleword"
> > "&& reload_completed"
> > [(parallel
> > [(set (reg:CCC FLAGS_REG)
> > - (ne:CCC (match_dup 1) (const_int 0)))
> > + (unspec:CCC [(match_dup 1) (const_int 0)] UNSPEC_CC_NE))
> > (set (match_dup 0) (neg:DWIH (match_dup 1)))])
> > (parallel
> > [(set (match_dup 2)
> > @@ -11499,7 +11500,8 @@ (define_peephole2
> > (match_operand:SWI48 1 "nonimmediate_gr_operand"))
> > (parallel
> > [(set (reg:CCC FLAGS_REG)
> > - (ne:CCC (match_operand:SWI48 2 "general_reg_operand") (const_int
> > 0)))
> > + (unspec:CCC [(match_operand:SWI48 2 "general_reg_operand")
> > + (const_int 0)] UNSPEC_CC_NE))
> > (set (match_dup 2) (neg:SWI48 (match_dup 2)))])
> > (parallel
> > [(set (match_dup 0)
> > @@ -11517,7 +11519,7 @@ (define_peephole2
> > && !reg_mentioned_p (operands[2], operands[1])"
> > [(parallel
> > [(set (reg:CCC FLAGS_REG)
> > - (ne:CCC (match_dup 2) (const_int 0)))
> > + (unspec:CCC [(match_dup 2) (const_int 0)] UNSPEC_CC_NE))
> > (set (match_dup 2) (neg:SWI48 (match_dup 2)))])
> > (parallel
> > [(set (match_dup 0)
> > @@ -11543,7 +11545,8 @@ (define_peephole2
> > (clobber (reg:CC FLAGS_REG))])
> > (parallel
> > [(set (reg:CCC FLAGS_REG)
> > - (ne:CCC (match_operand:SWI48 1 "general_reg_operand") (const_int
> > 0)))
> > + (unspec:CCC [(match_operand:SWI48 1 "general_reg_operand")
> > + (const_int 0)] UNSPEC_CC_NE))
> > (set (match_dup 1) (neg:SWI48 (match_dup 1)))])
> > (parallel
> > [(set (match_dup 0)
> > @@ -11559,7 +11562,7 @@ (define_peephole2
> > "REGNO (operands[0]) != REGNO (operands[1])"
> > [(parallel
> > [(set (reg:CCC FLAGS_REG)
> > - (ne:CCC (match_dup 1) (const_int 0)))
> > + (unspec:CCC [(match_dup 1) (const_int 0)] UNSPEC_CC_NE))
> > (set (match_dup 1) (neg:SWI48 (match_dup 1)))])
> > (parallel
> > [(set (match_dup 0)
> > @@ -11635,9 +11638,9 @@ (define_insn "*negsi_2_zext"
> >
> > (define_insn "*neg<mode>_ccc_1"
> > [(set (reg:CCC FLAGS_REG)
> > - (ne:CCC
> > - (match_operand:SWI 1 "nonimmediate_operand" "0")
> > - (const_int 0)))
> > + (unspec:CCC
> > + [(match_operand:SWI 1 "nonimmediate_operand" "0")
> > + (const_int 0)] UNSPEC_CC_NE))
> > (set (match_operand:SWI 0 "nonimmediate_operand" "=<r>m")
> > (neg:SWI (match_dup 1)))]
> > ""
> > @@ -11647,9 +11650,9 @@ (define_insn "*neg<mode>_ccc_1"
> >
> > (define_insn "*neg<mode>_ccc_2"
> > [(set (reg:CCC FLAGS_REG)
> > - (ne:CCC
> > - (match_operand:SWI 1 "nonimmediate_operand" "0")
> > - (const_int 0)))
> > + (unspec:CCC
> > + [(match_operand:SWI 1 "nonimmediate_operand" "0")
> > + (const_int 0)] UNSPEC_CC_NE))
> > (clobber (match_scratch:SWI 0 "=<r>"))]
> > ""
> > "neg{<imodesuffix>}\t%0"
> > @@ -11659,8 +11662,8 @@ (define_insn "*neg<mode>_ccc_2"
> > (define_expand "x86_neg<mode>_ccc"
> > [(parallel
> > [(set (reg:CCC FLAGS_REG)
> > - (ne:CCC (match_operand:SWI48 1 "register_operand")
> > - (const_int 0)))
> > + (unspec:CCC [(match_operand:SWI48 1 "register_operand")
> > + (const_int 0)] UNSPEC_CC_NE))
> > (set (match_operand:SWI48 0 "register_operand")
> > (neg:SWI48 (match_dup 1)))])])
> >
> > @@ -11686,8 +11689,9 @@ (define_insn "*negqi_ext<mode>_2"
> > ;; Negate with jump on overflow.
> > (define_expand "negv<mode>3"
> > [(parallel [(set (reg:CCO FLAGS_REG)
> > - (ne:CCO (match_operand:SWI 1 "register_operand")
> > - (match_dup 3)))
> > + (unspec:CCO
> > + [(match_operand:SWI 1 "register_operand")
> > + (match_dup 3)] UNSPEC_CC_NE))
> > (set (match_operand:SWI 0 "register_operand")
> > (neg:SWI (match_dup 1)))])
> > (set (pc) (if_then_else
> > @@ -11703,8 +11707,9 @@ (define_expand "negv<mode>3"
> >
> > (define_insn "*negv<mode>3"
> > [(set (reg:CCO FLAGS_REG)
> > - (ne:CCO (match_operand:SWI 1 "nonimmediate_operand" "0")
> > - (match_operand:SWI 2 "const_int_operand")))
> > + (unspec:CCO [(match_operand:SWI 1 "nonimmediate_operand" "0")
> > + (match_operand:SWI 2 "const_int_operand")]
> > + UNSPEC_CC_NE))
> > (set (match_operand:SWI 0 "nonimmediate_operand" "=<r>m")
> > (neg:SWI (match_dup 1)))]
> > "ix86_unary_operator_ok (NEG, <MODE>mode, operands)
> > @@ -11770,7 +11775,7 @@ (define_insn_and_split "*abs<dwi>2_doubleword"
> > "&& 1"
> > [(parallel
> > [(set (reg:CCC FLAGS_REG)
> > - (ne:CCC (match_dup 1) (const_int 0)))
> > + (unspec:CCC [(match_dup 1) (const_int 0)] UNSPEC_CC_NE))
> > (set (match_dup 2) (neg:DWIH (match_dup 1)))])
> > (parallel
> > [(set (match_dup 5)
> > @@ -11814,7 +11819,7 @@ (define_insn_and_split "*nabs<dwi>2_doubleword"
> > "&& 1"
> > [(parallel
> > [(set (reg:CCC FLAGS_REG)
> > - (ne:CCC (match_dup 1) (const_int 0)))
> > + (unspec:CCC [(match_dup 1) (const_int 0)] UNSPEC_CC_NE))
> > (set (match_dup 2) (neg:DWIH (match_dup 1)))])
> > (parallel
> > [(set (match_dup 5)
> > @@ -21456,7 +21461,7 @@ (define_split
> > (const_int 0))))]
> > [(set (reg:CCC FLAGS_REG)
> > - (ne:CCC (match_dup 1) (const_int 0)))
> > + (unspec:CCC [(match_dup 1) (const_int 0)] UNSPEC_CC_NE))
> Do we have a define_insn for this? It looks like a setcc to me.
It's not, nevermind.
> > (set (match_dup 0)
> > (neg:SWI (ltu:SWI (reg:CCC FLAGS_REG) (const_int 0))))])
> >
> > diff --git a/gcc/testsuite/gcc.target/i386/pr107172.c
> > b/gcc/testsuite/gcc.target/i386/pr107172.c
> > new file mode 100644
> > index 00000000000..d2c85f3f47c
> > --- /dev/null
> > +++ b/gcc/testsuite/gcc.target/i386/pr107172.c
> > @@ -0,0 +1,26 @@
> > +/* { dg-do run } */
> > +/* { dg-options "-O1 -ftree-vrp" } */
> > +
> > +int a, c, d;
> > +int
> > +main()
> > +{
> > + long e = 1;
> > + int f = a = 1;
> > +L1:
> > + if (a)
> > + a = 2;
> > + int h = e = ~e;
> > + c = -1;
> > + if (e >= a)
> > + goto L2;
> > + if (-1 > a)
> > + goto L1;
> > + if (a)
> > + f = -1;
> > +L2:
> > + d = (-f + d) & h;
> > + if (d)
> > + __builtin_abort();
> > + return 0;
> > +}
> > --
> > 2.37.3
> >
>
>
> --
> BR,
> Hongtao
--
BR,
Hongtao
