On Sun, Jan 24, 2016 at 03:19:35AM -0800, Richard Henderson wrote: > As Jakub notes in the PR, the representation for add_compare and > sub_compare were wrong. And several of the add_carryin patterns > were duplicates. > > This adds a CC_Cmode for which only the Carry bit is valid. > > The patch appears to generate moderately decent code. For gcc7 we > should look into why we'll prefer to mark an output REG_UNUSED > instead of matching the pattern with that output removed. This > results in continuing to use adds (though simplifying adc) after > we've proved that there will be no carry into the high part of an > adds+adc pair. > > Ok? > > > r~
Hi Richard, Some tiny nits below: > diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md > index 71fc514..363785e 100644 > --- a/gcc/config/aarch64/aarch64.md > +++ b/gcc/config/aarch64/aarch64.md > @@ -1755,6 +1755,44 @@ > [(set_attr "type" "alus_sreg,alus_imm,alus_imm")] > ) > > +(define_insn "*add<mode>3_compare1_cconly" I don't understand the naming scheme, it got me a wee bit confused with add<mode>3_compare0 and friends, where the 0 indicates a comparison with zero... > + [(set (reg:CC_C CC_REGNUM) > + (ne:CC_C > + (plus:<DWI> > + (zero_extend:<DWI> > + (match_operand:GPI 0 "aarch64_reg_or_zero" "%rZ,rZ,rZ")) > + (zero_extend:<DWI> > + (match_operand:GPI 1 "aarch64_plus_operand" "r,I,J"))) > + (zero_extend:<DWI> > + (plus:GPI (match_dup 0) (match_dup 1)))))] > + "" > + "@ > + cmn\\t%<w>0, %<w>1 > + cmn\\t%<w>0, %<w>1 > + cmp\\t%<w>0, #%n1" > + [(set_attr "type" "alus_sreg,alus_imm,alus_imm")] > +) > + > +(define_insn "add<mode>3_compare1" > + [(set (reg:CC_C CC_REGNUM) > + (ne:CC_C > + (plus:<DWI> > + (zero_extend:<DWI> > + (match_operand:GPI 1 "aarch64_reg_or_zero" "%rZ,rZ,rZ")) > + (zero_extend:<DWI> > + (match_operand:GPI 2 "aarch64_plus_operand" "r,I,J"))) > + (zero_extend:<DWI> > + (plus:GPI (match_dup 1) (match_dup 2))))) > + (set (match_operand:GPI 0 "register_operand" "=r,r,r") > + (plus:GPI (match_dup 1) (match_dup 2)))] > + "" > + "@ > + adds\\t%<w>0, %<w>1, %<w>2 > + adds\\t%<w>0, %<w>1, %<w>2 > + subs\\t%<w>0, %<w>1, #%n2" > + [(set_attr "type" "alus_sreg,alus_imm,alus_imm")] > +) > + > (define_insn "*adds_shift_imm_<mode>" > [(set (reg:CC_NZ CC_REGNUM) > (compare:CC_NZ <snip> > +;; Note that a single add with carry is matched by cinc, > +;; and the adc_reg and csel types are matched into the same > +;; pipelines by existing cores. I can't see us remembering to update this comment on pipeline models were it to ever become false. Maybe just drop it? > @@ -2440,13 +2427,53 @@ > [(set_attr "type" "alu_ext")] > ) > > -(define_insn "sub<mode>3_carryin" > - [(set > - (match_operand:GPI 0 "register_operand" "=r") > - (minus:GPI (minus:GPI > - (match_operand:GPI 1 "register_operand" "r") > - (ltu:GPI (reg:CC CC_REGNUM) (const_int 0))) > - (match_operand:GPI 2 "register_operand" "r")))] > +;; The hardware description is op1 + ~op2 + C. > +;; = op1 + (-op2 + 1) + (1 - !C) > +;; = op1 - op2 - 1 + 1 - !C > +;; = op1 - op2 - !C. > +;; We describe the later. s/later/latter/ Otherwise, this is OK. Thanks, James
