Skip to content

Commit d445cd7

Browse files
cfallincfallin
authored
Cranelift: add umul_overflow / smul_overflow flags folding cases. (#14254)
* Cranelift: add `umul_overflow` / `smul_overflow` flags folding cases. This stacks on top of #14228, and adds the equivalent cases for multiplies. This required some new cases in the `ProducesFlags` enum because, at least on aarch64, the lowerings are a little more complex than a single multiply instruction that sets flags (no such instruction exists on that ISA). The canonical idiom is instead to compare the product (full width) against the product extended-from-narrow-width; or, for 64-bit x 64-bit multiplies, use the separate high-half multiply instruction and compare against zero. This requires either two (8/16/32-bit case) or three (64-bit case) instructions to do the multiply and produce a flag for the overflow case. * Review feedback. * Add verification specs. * Verification fixes.
1 parent c4a69bb commit d445cd7

10 files changed

Lines changed: 1060 additions & 3 deletions

File tree

‎cranelift/codegen/src/isa/aarch64/inst.isle‎

Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4957,6 +4957,108 @@
49574957
(produces_flags_opportunistic_def producer sum_value)
49584958
(Cond.Hs))))
49594959

4960+
;; Use of the flags output of `umul_overflow`/`smul_overflow` can produce a
4961+
;; `Cond` and give an opportunistic def of the other output. The multiply does
4962+
;; not set the condition flags; the overflow is detected by comparing the
4963+
;; product against itself extended from the original type (or, for 64-bit
4964+
;; values, by comparing the high half of the product against zero). Hence the
4965+
;; `ProducesFlagsOpportunisticDef2` chains for the narrow cases. For 64-bit
4966+
;; multiplies, the low product and overflow flag are lowered independently,
4967+
;; since the lowerings have no overlap/shared computation.
4968+
4969+
;; madd out, a_uxt, b_uxt
4970+
;; cmp out, out, uxt{b,h}
4971+
(rule 5
4972+
(is_nonzero
4973+
(second_result umul @ (umul_overflow (fits_in_16 ty) a b)))
4974+
(if-let (first_result prod_value) umul)
4975+
(let ((a_uext Reg (put_in_reg_zext32 a))
4976+
(b_uext Reg (put_in_reg_zext32 b))
4977+
(dst WritableReg (temp_writable_reg $I64))
4978+
(producer ProducesFlags
4979+
(produces_flags_opportunistic_def2
4980+
(MInst.AluRRRR (ALUOp3.MAdd) (operand_size ty) dst a_uext b_uext (zero_reg))
4981+
dst
4982+
prod_value
4983+
(MInst.AluRRRExtend (ALUOp.SubS) (OperandSize.Size32)
4984+
(writable_zero_reg) dst dst
4985+
(lower_extend_op ty (ArgumentExtension.Uext))))))
4986+
(CondResult.Cond producer (Cond.Ne))))
4987+
4988+
;; umaddl out, a, b
4989+
;; cmp out, out, uxtw
4990+
(rule 6
4991+
(is_nonzero
4992+
(second_result umul @ (umul_overflow $I32 a b)))
4993+
(if-let (first_result prod_value) umul)
4994+
(let ((dst WritableReg (temp_writable_reg $I64))
4995+
(producer ProducesFlags
4996+
(produces_flags_opportunistic_def2
4997+
(MInst.AluRRRR (ALUOp3.UMAddL) (operand_size $I32) dst a b (zero_reg))
4998+
dst
4999+
prod_value
5000+
(MInst.AluRRRExtend (ALUOp.SubS) (OperandSize.Size64)
5001+
(writable_zero_reg) dst dst
5002+
(ExtendOp.UXTW)))))
5003+
(CondResult.Cond producer (Cond.Ne))))
5004+
5005+
;; umulh tmp, a, b
5006+
;; cmp tmp, #0
5007+
(rule 7
5008+
(is_nonzero
5009+
(second_result umul @ (umul_overflow $I64 a b)))
5010+
(let ((tmp Reg (umulh $I64 a b))
5011+
(producer ProducesFlags (cmp64_imm tmp (u8_into_imm12 0))))
5012+
(CondResult.Cond producer (Cond.Ne))))
5013+
5014+
;; madd out, a_sxt, b_sxt
5015+
;; cmp out, out, sxt{b,h}
5016+
(rule 8
5017+
(is_nonzero
5018+
(second_result smul @ (smul_overflow (fits_in_16 ty) a b)))
5019+
(if-let (first_result prod_value) smul)
5020+
(let ((a_sext Reg (put_in_reg_sext32 a))
5021+
(b_sext Reg (put_in_reg_sext32 b))
5022+
(dst WritableReg (temp_writable_reg $I64))
5023+
(producer ProducesFlags
5024+
(produces_flags_opportunistic_def2
5025+
(MInst.AluRRRR (ALUOp3.MAdd) (operand_size ty) dst a_sext b_sext (zero_reg))
5026+
dst
5027+
prod_value
5028+
(MInst.AluRRRExtend (ALUOp.SubS) (OperandSize.Size32)
5029+
(writable_zero_reg) dst dst
5030+
(lower_extend_op ty (ArgumentExtension.Sext))))))
5031+
(CondResult.Cond producer (Cond.Ne))))
5032+
5033+
;; smaddl out, a, b
5034+
;; cmp out, out, sxtw
5035+
(rule 9
5036+
(is_nonzero
5037+
(second_result smul @ (smul_overflow $I32 a b)))
5038+
(if-let (first_result prod_value) smul)
5039+
(let ((dst WritableReg (temp_writable_reg $I64))
5040+
(producer ProducesFlags
5041+
(produces_flags_opportunistic_def2
5042+
(MInst.AluRRRR (ALUOp3.SMAddL) (operand_size $I32) dst a b (zero_reg))
5043+
dst
5044+
prod_value
5045+
(MInst.AluRRRExtend (ALUOp.SubS) (OperandSize.Size64)
5046+
(writable_zero_reg) dst dst
5047+
(ExtendOp.SXTW)))))
5048+
(CondResult.Cond producer (Cond.Ne))))
5049+
5050+
;; smulh tmp, a, b
5051+
;; cmp tmp, out, asr #63
5052+
(rule 10
5053+
(is_nonzero
5054+
(second_result smul @ (smul_overflow $I64 a b)))
5055+
(if-let (first_result prod_value) smul)
5056+
(let ((prod Reg (put_in_reg prod_value))
5057+
(tmp Reg (smulh $I64 a b))
5058+
(producer ProducesFlags
5059+
(cmp_rr_shift_asr (OperandSize.Size64) tmp prod 63)))
5060+
(CondResult.Cond producer (Cond.Ne))))
5061+
49605062
(attr emit_icmp (veri chain))
49615063
(decl emit_icmp (IntCC Value Value) CondResult)
49625064

‎cranelift/codegen/src/isa/aarch64/lower.isle‎

Lines changed: 35 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2970,8 +2970,7 @@
29702970
;; bool (i.e., its only use(s) were folded into consumers such as a `brif` that
29712971
;; consume the flags directly) does not need a `cset`; emit just the `adds`.
29722972
(rule 3 (lower i @ (uadd_overflow (ty_32_or_64 ty) x y))
2973-
(if-let (second_result flags) i)
2974-
(if-let false (value_used flags))
2973+
(if-let false (second_result_used i))
29752974
(output_pair (add ty x y) (value_regs_invalid)))
29762975

29772976
;; For values smaller than a register, we do a normal `add` with both arguments
@@ -3049,6 +3048,24 @@
30493048

30503049
;;;; Rules for `umul_overflow` ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
30513050

3051+
;; The flags output of a `umul_overflow` that is not demanded as a
3052+
;; materialized bool (i.e., its only use(s) were folded into consumers such
3053+
;; as a `brif` that consume the flags directly) does not need the
3054+
;; `cmp`/`cset`; emit just the multiply.
3055+
(rule 5 (lower i @ (umul_overflow (fits_in_16 ty) a b))
3056+
(if-let false (second_result_used i))
3057+
(let ((a_uext Reg (put_in_reg_zext32 a))
3058+
(b_uext Reg (put_in_reg_zext32 b)))
3059+
(output_pair (madd ty a_uext b_uext (zero_reg)) (value_regs_invalid))))
3060+
3061+
(rule 4 (lower i @ (umul_overflow $I32 a b))
3062+
(if-let false (second_result_used i))
3063+
(output_pair (umaddl a b (zero_reg)) (value_regs_invalid)))
3064+
3065+
(rule 3 (lower i @ (umul_overflow $I64 a b))
3066+
(if-let false (second_result_used i))
3067+
(output_pair (madd $I64 a b (zero_reg)) (value_regs_invalid)))
3068+
30523069
;; uxt{b,h} a_ext, a
30533070
;; uxt{b,h} b_ext, b
30543071
;; mul out, a_ext, b_ext
@@ -3097,6 +3114,22 @@
30973114

30983115
;;;; Rules for `smul_overflow` ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
30993116

3117+
;; As for `umul_overflow` above: when the flags output is only consumed
3118+
;; folded (e.g., by a `brif`), emit just the multiply.
3119+
(rule 5 (lower i @ (smul_overflow (fits_in_16 ty) a b))
3120+
(if-let false (second_result_used i))
3121+
(let ((a_sext Reg (put_in_reg_sext32 a))
3122+
(b_sext Reg (put_in_reg_sext32 b)))
3123+
(output_pair (madd ty a_sext b_sext (zero_reg)) (value_regs_invalid))))
3124+
3125+
(rule 4 (lower i @ (smul_overflow $I32 a b))
3126+
(if-let false (second_result_used i))
3127+
(output_pair (smaddl a b (zero_reg)) (value_regs_invalid)))
3128+
3129+
(rule 3 (lower i @ (smul_overflow $I64 a b))
3130+
(if-let false (second_result_used i))
3131+
(output_pair (madd $I64 a b (zero_reg)) (value_regs_invalid)))
3132+
31003133
;; sxt{b,h} a_ext, a
31013134
;; sxt{b,h} b_ext, b
31023135
;; mul out, a_ext, b_ext

‎cranelift/codegen/src/isa/x64/inst.isle‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3886,6 +3886,46 @@
38863886
(produces_flags_opportunistic_def producer sum_value)
38873887
(CC.B))))
38883888

3889+
;; Using the flags output of `umul_overflow`/`smul_overflow` can produce a
3890+
;; `CC` and give an opportunistic def of the other output. The multiply
3891+
;; itself sets the overflow flag (OF for `imul`; OF==CF for the unsigned
3892+
;; `mul` of narrow types), so no extra compare is needed.
3893+
(rule 4
3894+
(is_nonzero
3895+
(second_result umul @ (umul_overflow _ x y @ (value_type (ty_int_ref_16_to_64 ty)))))
3896+
(if-let (first_result prod_value) umul)
3897+
(let ((producer ProducesFlags (x64_mul_lo_with_flags_paired ty false x y)))
3898+
(CondResult.CC
3899+
(produces_flags_opportunistic_def producer prod_value)
3900+
(CC.O))))
3901+
3902+
(rule 5
3903+
(is_nonzero
3904+
(second_result umul @ (umul_overflow _ x y @ (value_type $I8))))
3905+
(if-let (first_result prod_value) umul)
3906+
(let ((producer ProducesFlags (x64_mul8_with_flags_paired false x y)))
3907+
(CondResult.CC
3908+
(produces_flags_opportunistic_def producer prod_value)
3909+
(CC.O))))
3910+
3911+
(rule 6
3912+
(is_nonzero
3913+
(second_result smul @ (smul_overflow _ x y @ (value_type (ty_int_ref_16_to_64 ty)))))
3914+
(if-let (first_result prod_value) smul)
3915+
(let ((producer ProducesFlags (x64_mul_lo_with_flags_paired ty true x y)))
3916+
(CondResult.CC
3917+
(produces_flags_opportunistic_def producer prod_value)
3918+
(CC.O))))
3919+
3920+
(rule 7
3921+
(is_nonzero
3922+
(second_result smul @ (smul_overflow _ x y @ (value_type $I8))))
3923+
(if-let (first_result prod_value) smul)
3924+
(let ((producer ProducesFlags (x64_mul8_with_flags_paired true x y)))
3925+
(CondResult.CC
3926+
(produces_flags_opportunistic_def producer prod_value)
3927+
(CC.O))))
3928+
38893929
;; Like `is_nonzero` but with additional specializations for compare
38903930
;; operators. We break this out from `is_nonzero` because we want to
38913931
;; avoid unbounded recursion.

‎cranelift/codegen/src/isa/x64/lower.isle‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -219,6 +219,20 @@
219219

220220
;;;; Rules for `umul_overflow` ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
221221

222+
;; The flags output of a `umul_overflow` that is not demanded as a
223+
;; materialized bool (i.e., its only use(s) were folded into consumers such
224+
;; as a `brif` that consume the flags directly) does not need a `seto`;
225+
;; emit just the `mul`.
226+
(rule 4 (lower i @ (umul_overflow _ x y @ (value_type (ty_int_ref_16_to_64 ty))))
227+
(if-let (second_result flags) i)
228+
(if-let false (value_used flags))
229+
(output_pair (x64_mul ty false x y) (value_regs_invalid)))
230+
231+
(rule 5 (lower i @ (umul_overflow _ x y @ (value_type $I8)))
232+
(if-let (second_result flags) i)
233+
(if-let false (value_used flags))
234+
(output_pair (value_reg (x64_mul8 false x y)) (value_regs_invalid)))
235+
222236
(rule 2 (lower (umul_overflow _ x y @ (value_type $I8)))
223237
(construct_overflow_op (CC.O) (x64_mul8_with_flags_paired false x y)))
224238

@@ -227,6 +241,18 @@
227241

228242
;;;; Rules for `smul_overflow` ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
229243

244+
;; As for `umul_overflow` above: when the flags output is only consumed
245+
;; folded (e.g., by a `brif`), emit just the `imul`.
246+
(rule 4 (lower i @ (smul_overflow _ x y @ (value_type (ty_int_ref_16_to_64 ty))))
247+
(if-let (second_result flags) i)
248+
(if-let false (value_used flags))
249+
(output_pair (x64_mul ty true x y) (value_regs_invalid)))
250+
251+
(rule 5 (lower i @ (smul_overflow _ x y @ (value_type $I8)))
252+
(if-let (second_result flags) i)
253+
(if-let false (value_used flags))
254+
(output_pair (value_reg (x64_mul8 true x y)) (value_regs_invalid)))
255+
230256
(rule 2 (lower (smul_overflow _ x y @ (value_type $I8)))
231257
(construct_overflow_op (CC.O) (x64_mul8_with_flags_paired true x y)))
232258

‎cranelift/codegen/src/machinst/isle.rs‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -213,6 +213,12 @@ macro_rules! isle_lower_prelude_methods {
213213
.copied()
214214
}
215215

216+
#[inline]
217+
fn second_result_used(&mut self, inst: Inst) -> bool {
218+
let second_result = self.lower_ctx.dfg().inst_results(inst).get(1).copied();
219+
second_result.is_some_and(|value| self.lower_ctx.value_lowered_used(value))
220+
}
221+
216222
#[inline]
217223
fn inst_data_value(&mut self, inst: Inst) -> (Type, InstructionData) {
218224
let ty = match self.first_result(inst) {

0 commit comments

Comments
 (0)