On Wed, Oct 14, 2020 at 3:49 PM Jakub Jelinek <ja...@redhat.com> wrote:
>
> On Wed, Oct 14, 2020 at 03:17:03PM +0200, Uros Bizjak wrote:
> > > +(define_insn_and_split "*setcc_qi_addqi3_cconly_overflow_1_<mode>"
> > > + [(set (reg:CCC FLAGS_REG)
> > > + (compare:CCC (neg:QI (geu:QI (reg:CC_CCC FLAGS_REG) (const_int
> > > 0)))
> > > + (ltu:QI (reg:CC_CCC FLAGS_REG) (const_int 0))))]
> > > + "ix86_pre_reload_split ()"
> > > + "#"
> > > + "&& 1"
> > > + [(const_int 0)])
> > >
> >
> > Hmm... does the above really represent a NOP?
>
> It is just what combine.c + simplify-rtx.c make out of this.
> We have:
> (insn 10 9 11 2 (set (reg:QI 88 [ _31 ])
> (ltu:QI (reg:CCC 17 flags)
> (const_int 0 [0]))) "include/adxintrin.h":69:10 785 {*setcc_qi}
> (expr_list:REG_DEAD (reg:CCC 17 flags)
> (nil)))
> and
> (insn 17 15 18 2 (parallel [
> (set (reg:CCC 17 flags)
> (compare:CCC (plus:QI (reg:QI 88 [ _31 ])
> (const_int -1 [0xffffffffffffffff]))
> (reg:QI 88 [ _31 ])))
> (clobber (scratch:QI))
> ]) "include/adxintrin.h":69:10 350 {*addqi3_cconly_overflow_1}
> (expr_list:REG_DEAD (reg:QI 88 [ _31 ])
> (nil)))
> So when substituting (reg:QI 88) for (ltu:QI flags 0), we initially get:
> (compare:CCC (plus:QI (ltu:QI (reg:CCC 17 flags) (const_int [0])) (const_int
> -1 [0xffffffffffffffff]))
> (ltu:QI (reg:CCC 17 flags) (const_int [0])))
> On this triggers simplify_binary_operation_1 rule:
> /* (plus (comparison A B) C) can become (neg (rev-comp A B)) if
> C is 1 and STORE_FLAG_VALUE is -1 or if C is -1 and STORE_FLAG_VALUE
> is 1. */
> if (COMPARISON_P (op0)
> && ((STORE_FLAG_VALUE == -1 && trueop1 == const1_rtx)
> || (STORE_FLAG_VALUE == 1 && trueop1 == constm1_rtx))
> && (reversed = reversed_comparison (op0, mode)))
> return
> simplify_gen_unary (NEG, mode, reversed, mode);
> As STORE_FLAG_VALUE is 1 on i386, it triggers for that -1.
>
> Now, in CCCmode we have just 2 possible values, 0 and 1, CF clear and CF set.
> So, either (ltu:QI (reg:CCC 17 flags) (const_int 0 [0])) is 0, in that case
> (geu:QI (reg:CCC 17 flags) (const_int 0 [0])) is 1 and so
> (compare:CCC (neg:QI (geu:QI (reg:CCC FLAGS_REG) (const_int 0)))
> (ltu:QI (reg:CCC FLAGS_REG) (const_int 0))))
> is (compare:CCC (neg:QI (const_int 1 [0x1])) (const_int 0 [0]))
> which is (compare:CCC (const_int -1 [0xffffffffffffffff]) (const_int 0 [0]))
> Or (ltu:QI (reg:CCC 17 flags) (const_int 0 [0])) is 1, in that case
> (geu:QI (reg:CCC 17 flags) (const_int 0 [0])) is 0 and so
> (compare:CCC (neg:QI (geu:QI (reg:CCC FLAGS_REG) (const_int 0)))
> (ltu:QI (reg:CCC FLAGS_REG) (const_int 0))))
> is (compare:CCC (neg:QI (const_int 0 [0x0])) (const_int 1 [0x1]))
> which is (compare:CCC (const_int 0 [0]) (const_int 1 [0x1]))
> As CCCmode flags can be only used in LTU or GEU comparisons, we are asking if
> 0xffU < 0 (false, CF clear) or 0 < 1 (true, CF set).
>
> So I think the pattern is meaningful and is really a nop.
Phew ... it took me some time to wrap my mind around the above logic.
The explanation clears my concerns, and also answers the question if
this simplification should be implemented in some generic,
target-independant way. No, due to all the target-dependant stuff,
mentioned in the explanation.
OK for mainline.
Thanks,
Uros.
