On Sun, Nov 22, 2015 at 11:38 AM, Richard Henderson <r...@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.
