Re: [PATCH v2] Add uaddv_optab, usubv4_optab

[Uros Bizjak <ubizjak at gmail dot com>]

On Sun, Nov 22, 2015 at 11:38 AM, Richard Henderson <rth@redhat.com> wrote:
> The full round of testing from v1 turned up a couple of problems.
>
> One of which I believe I've worked around in the i386 backend, but I believe
> to be a latent problem within combine.
>
> With the following patch, disable the add<mode>3_*_overflow_2 patterns.
> Then compile c-c++-common/torture/builtin-arith-overflow-4.c with -O2 and
> you'll see
>
>  t151_2add:
>        testb   %dil, %dil
>        leal    -1(%rdi), %eax
>        jne     .L644
>
> which is incorrect.  Combine has looked through two comparisons, seen the NE
> in the second comparison, and then converted a CCCmode compare to a CCZmode
> compare.
>
> I wonder if the code in combine.c simplify_set which calls SELECT_CC_MODE
> ought to be throwing away existing MODE_CC data.  Ought we verify that the
> newly selected CC mode is cc_modes_compatible with the existing?  That would
> certainly work when we begin with the "usual" fully general CCmode compare,
> optimizing to CCZmode, but would then reject moving between CCCmode and
> CCZmode.
>
> That said, the addition of the new _overflow_2 patterns allows CSE to
> simplify the expressions earlier, which avoids the problem in combine.
>
> Ok?

> * optabs.def (uaddv4_optab, usubv4_optab): New.
> * internal-fn.c (expand_addsub_overflow): Use them.
> * doc/md.texi (Standard Names): Add uaddv<m>4, usubv<m>4.
>
> * config/i386/i386.c (ix86_cc_mode): Extend add overflow check
> to reversed operands.
> * config/i386/i386.md (uaddv<SWI>4, usubv<SWI>4): New.
> (*add<SWI>3_cconly_overflow_1): Rename *add<SWI>3_cconly_overflow.
> (*add<SWI>3_cc_overflow_1): Rename *add<SWI>3_cc_overflow.
> (*addsi3_zext_cc_overflow_1): Rename *add3_zext_cc_overflow.
> (*add<SWI>3_cconly_overflow_2): New.
> (*add<SWI>3_cc_overflow_2): New.
> (*addsi3_zext_cc_overflow_2): New.

x86 parts are OK with a small change, see below.

+(define_expand "usubv<mode>4"
+  [(parallel [(set (reg:CC FLAGS_REG)
+   (compare:CC
+     (match_operand:SWI 1 "nonimmediate_operand")
+     (match_operand:SWI 2 "<general_operand>")))
+      (set (match_operand:SWI 0 "register_operand")
+   (minus:SWI (match_dup 1) (match_dup 2)))])
+   (set (pc) (if_then_else
+       (ltu (reg:CC FLAGS_REG) (const_int 0))
+       (label_ref (match_operand 3))
+       (pc)))]
+  ""
+{
+  ix86_fixup_binary_operands_no_copy (MINUS, <MODE>mode, operands);
+})

Please use quotes instead of braces in the one-line preparation
statement here and in the other place.

Thanks,
Uros.