On 8/5/25 19:15, Richard Sandiford wrote:
Should we also add a clobber to:

;; For a 24-bit immediate CST we can optimize the compare for equality
;; and branch sequence from:
;;      mov     x0, #imm1
;;      movk    x0, #imm2, lsl 16 /* x0 contains CST.  */
;;      cmp     x1, x0
;;      b<ne,eq> .Label
;; into the shorter:
;;      sub     x0, x1, #(CST & 0xfff000)
;;      subs    x0, x0, #(CST & 0x000fff)
;;      b<ne,eq> .Label
(define_insn_and_split "*aarch64_bcond_wide_imm<GPI:mode>"
   [(set (pc) (if_then_else (EQL (match_operand:GPI 0 "register_operand" "r")
                                (match_operand:GPI 1 "aarch64_imm24" "n"))
                           (label_ref:P (match_operand 2))
                           (pc)))]
   "!aarch64_move_imm (INTVAL (operands[1]), <GPI:MODE>mode)
    && !aarch64_plus_operand (operands[1], <GPI:MODE>mode)
    && !reload_completed"
   "#"
   "&& true"
   [(const_int 0)]
   {
     HOST_WIDE_INT lo_imm = UINTVAL (operands[1]) & 0xfff;
     HOST_WIDE_INT hi_imm = UINTVAL (operands[1]) & 0xfff000;
     rtx tmp = gen_reg_rtx (<GPI:MODE>mode);
     emit_insn (gen_add<GPI:mode>3 (tmp, operands[0], GEN_INT (-hi_imm)));
     emit_insn (gen_add<GPI:mode>3_compare0 (tmp, tmp, GEN_INT (-lo_imm)));
     rtx cc_reg = gen_rtx_REG (CC_NZmode, CC_REGNUM);
     rtx cmp_rtx = gen_rtx_fmt_ee (<EQL:CMP>, <GPI:MODE>mode,
                                  cc_reg, const0_rtx);
     emit_jump_insn (gen_aarch64_bcond (cmp_rtx, cc_reg, operands[2]));
     DONE;
   }
)

?  It looks like there should always have been one, since the pattern
clobbers CC internally.  But maybe that would defeat the define_split,
if it was primarily intended to be a 3->2 combine splitter rather than
an insn splitter.

As written, yes, this really should have a cc clobber. That said, I'm a little confused why we'd want to use SUBS+B.{EQ,NE} instead of SUB+CB{Z,NZ}.


Having a define_insn conditional on !reload_completed seems suspicious,
but that's for another day.

Indeed.


r~

Reply via email to