On Fri, Apr 07, 2017 at 04:34:44PM +0200, Ulrich Weigand wrote:
> > +; Peephole to combine a load-and-test from volatile memory which combine
> > does
> > +; not do.
> > +(define_peephole2
> > + [(set (match_operand:GPR 0 "register_operand")
> > + (match_operand:GPR 2 "memory_operand"))
> > + (set (reg CC_REGNUM)
> > + (compare (match_dup 0) (match_operand:GPR 1 "const0_operand")))]
> > + "s390_match_ccmode(insn, CCSmode) && TARGET_EXTIMM
> > + && GENERAL_REG_P (operands[0])
> > + && satisfies_constraint_T (operands[2])"
> > + [(parallel
> > + [(set (reg:CCS CC_REGNUM)
> > + (compare:CCS (match_dup 2) (match_dup 1)))
> > + (set (match_dup 0) (match_dup 2))])])
>
> Still wondering why this is necessary.
It's necessary vecause Combine refuses to match anything that
contains a volatile memory reference, using a global flag for
Recog.
> > @@ -6518,13 +6533,30 @@
> > [(parallel
> > [(set (match_operand:SI 0 "register_operand" "")
> > (match_operator:SI 1 "s390_eqne_operator"
> > - [(match_operand:CCZ1 2 "register_operand")
> > + [(match_operand 2 "cc_reg_operand")
> > (match_operand 3 "const0_operand")]))
> > (clobber (reg:CC CC_REGNUM))])]
> > ""
> > - "emit_insn (gen_sne (operands[0], operands[2]));
> > - if (GET_CODE (operands[1]) == EQ)
> > - emit_insn (gen_xorsi3 (operands[0], operands[0], const1_rtx));
> > + "machine_mode mode = GET_MODE (operands[2]);
> > + if (TARGET_Z196)
> > + {
> > + rtx cond, ite;
> > +
> > + if (GET_CODE (operands[1]) == NE)
> > + cond = gen_rtx_NE (VOIDmode, operands[2], const0_rtx);
> > + else
> > + cond = gen_rtx_EQ (VOIDmode, operands[2], const0_rtx);
> > + ite = gen_rtx_IF_THEN_ELSE (SImode, cond, const1_rtx, const0_rtx);
> > + emit_insn (gen_rtx_SET (operands[0], ite));
> > + }
> > + else
> > + {
> > + if (mode != CCZ1mode)
> > + FAIL;
> > + emit_insn (gen_sne (operands[0], operands[2]));
> > + if (GET_CODE (operands[1]) == EQ)
> > + emit_insn (gen_xorsi3 (operands[0], operands[0], const1_rtx));
> > + }
> > DONE;")
>
> >From what I can see in the rest of the patch, none of the CS changes now
> actually *rely* on this change to cstorecc4 ... s390_expand_cs_tdsi only
> calls cstorecc4 on !TARGET_Z196, where the above change is a no-op, and
> in the TARGET_Z196 case it deliberates does *not* use cstorecc4.
You're right. After all the refactoring, this part of the patch
has become unused.
> Now, in general this improvement to cstorecc4 is of course valuable
> in itself. But I think at this point it might be better to separate
> this out into an independent patch (and measure its effect separately).
Ciao
Dominik ^_^ ^_^
--
Dominik Vogt
IBM Germany
