On Tue, Aug 6, 2019 at 11:03 PM Richard Earnshaw (lists) <richard.earns...@arm.com> wrote: > > On 06/08/2019 18:39, Uros Bizjak wrote: > > Hello! > > > >> On Tue, Aug 06, 2019 at 05:49:17PM +0100, Richard Earnshaw (lists) wrote: > >>> On 06/08/2019 17:39, Segher Boessenkool wrote: > >>>>>> What's wrong with describing the canonical form in your MD? You'll > >>>>>> need > >>>>>> some reversed condition code thingy, but that's it? > >>>>> > >>>>> It doesn't describe what the instruction does. The negation has a side > >>>>> effect of setting the flags, but the flags are swapped because the > >>>>> side-effect comparison is swapped from a normal compare. As I > >>>>> mentioned, SELECT_CC_MODE doesn't help because it can't see the context > >>>>> and the comparison just looks 'normal'. > >>>> > >>>> Sure, and we can work on making combine do what you want, but your > >>>> existing > >>>> pattern is *incorrect*. It needs fixing, and probably before we do other > >>>> things. > >>> > >>> Why is it incorrect? It's not canonical, sure. But the cannonical form > >>> does NOT describe what the instruction does. > >> > >> More precisely: not having the canonical form of this in your MD is what > >> is incorrect. > > > > There is TARGET_CANONICALIZE_COMPARISON target hook that can solve the > > issue here. Please see x86, where we canonicalize > > *cmp<X87MODEF:mode>_<SWI24:mode>_i387 via > > ix86_canonicalize_comparison. As said in the comment: > > > > /* The order of operands in x87 ficom compare is forced by combine in > > simplify_comparison () function. Float operator is treated as RTX_OBJ > > with a precedence over other operators and is always put in the first > > place. Swap condition and operands to match ficom instruction. */ > > > > The issue looks similar to me. > > > > That hook can't help as it can't see anything other than the operands of > the compare, and from the operands it looks as though the operands > should be swapped. To correctly canonicalize this you need the whole > parallel, or some other hint that the operation is a side-effect of > another operation.
I see. FYI, there is the same problem in i386.md with "*neg<mode>2_cmpz" --cut here-- ;; The problem with neg is that it does not perform (compare x 0), ;; it really performs (compare 0 x), which leaves us with the zero ;; flag being the only useful item. (define_insn "*neg<mode>2_cmpz" [(set (reg:CCZ FLAGS_REG) (compare:CCZ (neg:SWI (match_operand:SWI 1 "nonimmediate_operand" "0")) (const_int 0))) (set (match_operand:SWI 0 "nonimmediate_operand" "=<r>m") (neg:SWI (match_dup 1)))] --cut here-- However, when trying to generate the pattern using following testcase: --cut here-- void foo (void); void bar (void); int test (int a, int b) { int r; if (r = -b) foo (); else bar (); return r; } --cut here-- combine doesn't even consider the combination, since the compare looks at the original value: (insn 7 4 8 2 (parallel [ (set (reg/v:SI 82 [ <retval> ]) (neg:SI (reg/v:SI 84 [ b ]))) (clobber (reg:CC 17 flags)) ]) "neg.c":9:9 461 {*negsi2_1} (expr_list:REG_UNUSED (reg:CC 17 flags) (nil))) (insn 8 7 9 2 (set (reg:CCZ 17 flags) (compare:CCZ (reg/v:SI 84 [ b ]) (const_int 0 [0]))) "neg.c":9:6 7 {*cmpsi_ccno_1} (expr_list:REG_DEAD (reg/v:SI 84 [ b ]) (nil))) probably due to tree optimizers that give us: r_3 = -b_2(D); if (b_2(D) != 0) goto <bb 3>; [50.00%] else goto <bb 4>; [50.00%] Uros.