Fix subtle ISLE extractor issue leading to incorrect matching in *mul_overflow lowering.

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

@@ -4992,7 +4992,7 @@
 ;; give an opportunistic def of the other output.
 (rule 4
       (is_nonzero
-       (second_result uadd @ (uadd_overflow (ty_32_or_64 ty) x y)))
+       (is_second_result uadd @ (uadd_overflow (ty_32_or_64 ty) x y)))
       (if-let (first_result sum_value) uadd)
       (let ((producer ProducesFlags (alu_rrr_with_flags_paired ty x y (ALUOp.AddS))))
         (CondResult.Cond
@@ -5012,7 +5012,7 @@
 ;; cmp out, out, uxt{b,h}
 (rule 5
       (is_nonzero
-       (second_result umul @ (umul_overflow (fits_in_16 ty) a b)))
+       (is_second_result umul @ (umul_overflow (fits_in_16 ty) a b)))
       (if-let (first_result prod_value) umul)
       (let ((a_uext Reg (put_in_reg_zext32 a))
             (b_uext Reg (put_in_reg_zext32 b))
@@ -5031,7 +5031,7 @@
 ;; cmp out, out, uxtw
 (rule 6
       (is_nonzero
-       (second_result umul @ (umul_overflow $I32 a b)))
+       (is_second_result umul @ (umul_overflow $I32 a b)))
       (if-let (first_result prod_value) umul)
       (let ((dst WritableReg (temp_writable_reg $I64))
             (producer ProducesFlags
@@ -5048,7 +5048,7 @@
 ;; cmp tmp, #0
 (rule 7
       (is_nonzero
-       (second_result umul @ (umul_overflow $I64 a b)))
+       (is_second_result umul @ (umul_overflow $I64 a b)))
       (let ((tmp Reg (umulh $I64 a b))
             (producer ProducesFlags (cmp64_imm tmp (u8_into_imm12 0))))
         (CondResult.Cond producer (Cond.Ne))))
@@ -5057,7 +5057,7 @@
 ;; cmp out, out, sxt{b,h}
 (rule 8
       (is_nonzero
-       (second_result smul @ (smul_overflow (fits_in_16 ty) a b)))
+       (is_second_result smul @ (smul_overflow (fits_in_16 ty) a b)))
       (if-let (first_result prod_value) smul)
       (let ((a_sext Reg (put_in_reg_sext32 a))
             (b_sext Reg (put_in_reg_sext32 b))
@@ -5076,7 +5076,7 @@
 ;; cmp out, out, sxtw
 (rule 9
       (is_nonzero
-       (second_result smul @ (smul_overflow $I32 a b)))
+       (is_second_result smul @ (smul_overflow $I32 a b)))
       (if-let (first_result prod_value) smul)
       (let ((dst WritableReg (temp_writable_reg $I64))
             (producer ProducesFlags
@@ -5093,7 +5093,7 @@
 ;; cmp tmp, out, asr #63
 (rule 10
       (is_nonzero
-       (second_result smul @ (smul_overflow $I64 a b)))
+       (is_second_result smul @ (smul_overflow $I64 a b)))
       (if-let (first_result prod_value) smul)
       (let ((prod Reg (put_in_reg prod_value))
             (tmp Reg (smulh $I64 a b))

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

@@ -3879,7 +3879,7 @@
 ;; opportunistic def of the other output.
 (rule 3
       (is_nonzero
-       (second_result uadd @ (uadd_overflow (fits_in_64 ty) x y)))
+       (is_second_result uadd @ (uadd_overflow (fits_in_64 ty) x y)))
       (if-let (first_result sum_value) uadd)
       (let ((producer ProducesFlags (x64_add_with_flags_paired ty x y)))
         (CondResult.CC
@@ -3892,7 +3892,7 @@
 ;; `mul` of narrow types), so no extra compare is needed.
 (rule 4
       (is_nonzero
-       (second_result umul @ (umul_overflow _ x y @ (value_type (ty_int_ref_16_to_64 ty)))))
+       (is_second_result umul @ (umul_overflow _ x y @ (value_type (ty_int_ref_16_to_64 ty)))))
       (if-let (first_result prod_value) umul)
       (let ((producer ProducesFlags (x64_mul_lo_with_flags_paired ty false x y)))
         (CondResult.CC
@@ -3901,7 +3901,7 @@
 
 (rule 5
       (is_nonzero
-       (second_result umul @ (umul_overflow _ x y @ (value_type $I8))))
+       (is_second_result umul @ (umul_overflow _ x y @ (value_type $I8))))
       (if-let (first_result prod_value) umul)
       (let ((producer ProducesFlags (x64_mul8_with_flags_paired false x y)))
         (CondResult.CC
@@ -3910,7 +3910,7 @@
 
 (rule 6
       (is_nonzero
-       (second_result smul @ (smul_overflow _ x y @ (value_type (ty_int_ref_16_to_64 ty)))))
+       (is_second_result smul @ (smul_overflow _ x y @ (value_type (ty_int_ref_16_to_64 ty)))))
       (if-let (first_result prod_value) smul)
       (let ((producer ProducesFlags (x64_mul_lo_with_flags_paired ty true x y)))
         (CondResult.CC
@@ -3919,7 +3919,7 @@
 
 (rule 7
       (is_nonzero
-       (second_result smul @ (smul_overflow _ x y @ (value_type $I8))))
+       (is_second_result smul @ (smul_overflow _ x y @ (value_type $I8))))
       (if-let (first_result prod_value) smul)
       (let ((producer ProducesFlags (x64_mul8_with_flags_paired true x y)))
         (CondResult.CC

cranelift/codegen/src/machinst/isle.rs

@@ -213,6 +213,20 @@ macro_rules! isle_lower_prelude_methods {
                 .copied()
         }
 
+        #[inline]
+        fn is_second_result(&mut self, val: Value) -> Option<Value> {
+            let inst = self.def_inst(val)?;
+            let is_match = self
+                .lower_ctx
+                .dfg()
+                .inst_results(inst)
+                .iter()
+                .skip(1)
+                .next()
+                == Some(&val);
+            if is_match { Some(val) } else { None }
+        }
+
         #[inline]
         fn second_result_used(&mut self, inst: Inst) -> bool {
             let second_result = self.lower_ctx.dfg().inst_results(inst).get(1).copied();

cranelift/codegen/src/prelude_lower.isle

@@ -301,10 +301,23 @@
 (extern extractor first_result first_result)
 
 ;; Extract the second result value of the given instruction.
+;;
+;; Be careful using this: if used to match on a *`Value`*, the
+;; auto-converter will take that `Value` to its defining `Inst`, then
+;; the inner result of this extractor will be the second def of the
+;; value; in other words, it will *fetch* the second result but will
+;; not *assert/only match if* the initial `Value` is the second
+;; result. Use `is_second_result` for that.
 (spec (second_result value) (provide (= result value)))
 (decl second_result (Value) Inst)
 (extern extractor second_result second_result)
 
+;; Determine whether the given value is the second result of its
+;; defining instruction.
+(spec (is_second_result value) (provide (= result value)))
+(decl is_second_result (Value) Value)
+(extern extractor is_second_result is_second_result)
+
 ;; Returns whether an instruction's second result still has uses to serve in
 ;; the lowering scan. Overflow operations use this while lowering their first
 ;; (non-boolean) result.

cranelift/filetests/filetests/runtests/issue-14293.clif

@@ -0,0 +1,15 @@
+test run
+set opt_level=speed_and_size
+target x86_64
+target aarch64
+
+function %a(i64) -> i8 {
+block0(v0: i64):
+    v1, v2 = smul_overflow.i64 v0, v0
+    v3 = iconst.i64 0
+    v4 = icmp eq v1, v3
+    return v4
+}
+
+; run: %a(0) == 1
+; run: %a(8192) == 0