pulley: Flesh out conditional registers (bytecodealliance#9793)

* pulley: Flesh out conditional registers This commit redefines previous Pulley instructions working with conditional values and results to always operate on the low 32-bits of a register rather than the full 64-bit width of integer registers. This should help 32-bit platforms work with just a word and avoid an extraneous load of top bits that are likely always zero. The previous `br_if` and `br_if_not` instructions now have a "32" suffix to make it clear that they're only operating on 32-bit register widths. Additionally the `xeq32` family of instructions (compare-and-set) now all only define the low 32-bits of the destination register. Finally, lowerings of `select` in CLIF were added for integer and floating-point registers. cc bytecodealliance#9783 * Fix interpreter tests
wasmfx · Dec 11, 2024 · 9aa048b · 9aa048b
1 parent fc9ec7a
commit 9aa048b
Show file tree

Hide file tree

Showing 16 changed files with 262 additions and 148 deletions.
diff --git a/cranelift/codegen/src/isa/pulley_shared/abi.rs b/cranelift/codegen/src/isa/pulley_shared/abi.rs
@@ -532,16 +532,15 @@ where
         _isa_flags: &PulleyFlags,
     ) -> u32 {
         match rc {
-            // Spilling an integer register requires spilling 8 bytes, and spill
-            // slots are defined in terms of "word bytes" or the size of a
-            // pointer. That means on 32-bit pulley we need to take up two spill
-            // slots for integers where on 64-bit pulley we need to only take up
-            // one spill slot for integers.
-            RegClass::Int => match P::pointer_width() {
+            // Spilling an integer or float register requires spilling 8 bytes,
+            // and spill slots are defined in terms of "word bytes" or the size
+            // of a pointer. That means on 32-bit pulley we need to take up two
+            // spill slots where on 64-bit pulley we need to only take up one
+            // spill slot for integers.
+            RegClass::Int | RegClass::Float => match P::pointer_width() {
                 PointerWidth::PointerWidth32 => 2,
                 PointerWidth::PointerWidth64 => 1,
             },
-            RegClass::Float => todo!(),
             RegClass::Vector => unreachable!(),
         }
     }

diff --git a/cranelift/codegen/src/isa/pulley_shared/inst.isle b/cranelift/codegen/src/isa/pulley_shared/inst.isle
@@ -61,7 +61,7 @@
     (Jump (label MachLabel))
 
     ;; Jump to `then` if `c` is nonzero, otherwise to `else`.
-    (BrIf (c XReg) (taken MachLabel) (not_taken MachLabel))
+    (BrIf32 (c XReg) (taken MachLabel) (not_taken MachLabel))
 
     ;; Compare-and-branch macro ops.
     (BrIfXeq32 (src1 XReg) (src2 XReg) (taken MachLabel) (not_taken MachLabel))
@@ -372,9 +372,9 @@
 (rule (pulley_jump label)
       (SideEffectNoResult.Inst (MInst.Jump label)))
 
-(decl pulley_br_if (XReg MachLabel MachLabel) SideEffectNoResult)
-(rule (pulley_br_if c taken not_taken)
-      (SideEffectNoResult.Inst (MInst.BrIf c taken not_taken)))
+(decl pulley_br_if32 (XReg MachLabel MachLabel) SideEffectNoResult)
+(rule (pulley_br_if32 c taken not_taken)
+      (SideEffectNoResult.Inst (MInst.BrIf32 c taken not_taken)))
 
 (decl pulley_br_if_xeq32 (XReg XReg MachLabel MachLabel) SideEffectNoResult)
 (rule (pulley_br_if_xeq32 a b taken not_taken)
@@ -431,11 +431,3 @@
 
 (decl gen_call_indirect (SigRef Value ValueSlice) InstOutput)
 (extern constructor gen_call_indirect gen_call_indirect)
-
-;;;; Helpers for Sign/Zero extension ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
-
-(decl zext (Value) XReg)
-(rule (zext val @ (value_type $I64)) val)
-(rule (zext val @ (value_type $I32)) (pulley_zext32 val))
-(rule (zext val @ (value_type $I16)) (pulley_zext16 val))
-(rule (zext val @ (value_type $I8)) (pulley_zext8 val))
diff --git a/cranelift/codegen/src/isa/pulley_shared/inst/emit.rs b/cranelift/codegen/src/isa/pulley_shared/inst/emit.rs
@@ -247,7 +247,7 @@ fn pulley_emit<P>(
             enc::jump(sink, 0x00000000);
         }
 
-        Inst::BrIf {
+        Inst::BrIf32 {
             c,
             taken,
             not_taken,
@@ -258,14 +258,14 @@ fn pulley_emit<P>(
 
             sink.use_label_at_offset(taken_start, *taken, LabelUse::Jump(2));
             let mut inverted = SmallVec::<[u8; 16]>::new();
-            enc::br_if_not(&mut inverted, c, 0x00000000);
+            enc::br_if_not32(&mut inverted, c, 0x00000000);
             debug_assert_eq!(
                 inverted.len(),
                 usize::try_from(taken_end - *start_offset).unwrap()
             );
 
             sink.add_cond_branch(*start_offset, taken_end, *taken, &inverted);
-            enc::br_if(sink, c, 0x00000000);
+            enc::br_if32(sink, c, 0x00000000);
             debug_assert_eq!(sink.cur_offset(), taken_end);
 
             // If not taken.

diff --git a/cranelift/codegen/src/isa/pulley_shared/inst/mod.rs b/cranelift/codegen/src/isa/pulley_shared/inst/mod.rs
@@ -164,7 +164,7 @@ fn pulley_get_operands(inst: &mut Inst, collector: &mut impl OperandVisitor) {
 
         Inst::Jump { .. } => {}
 
-        Inst::BrIf {
+        Inst::BrIf32 {
             c,
             taken: _,
             not_taken: _,
@@ -426,7 +426,7 @@ where
             }
             | Inst::Rets { .. } => MachTerminator::Ret,
             Inst::Jump { .. } => MachTerminator::Uncond,
-            Inst::BrIf { .. }
+            Inst::BrIf32 { .. }
             | Inst::BrIfXeq32 { .. }
             | Inst::BrIfXneq32 { .. }
             | Inst::BrIfXslt32 { .. }
@@ -651,15 +651,15 @@ impl Inst {
 
             Inst::Jump { label } => format!("jump {}", label.to_string()),
 
-            Inst::BrIf {
+            Inst::BrIf32 {
                 c,
                 taken,
                 not_taken,
             } => {
                 let c = format_reg(**c);
                 let taken = taken.to_string();
                 let not_taken = not_taken.to_string();
-                format!("br_if {c}, {taken}; jump {not_taken}")
+                format!("br_if32 {c}, {taken}; jump {not_taken}")
             }
 
             Inst::BrIfXeq32 {

diff --git a/cranelift/codegen/src/isa/pulley_shared/lower.isle b/cranelift/codegen/src/isa/pulley_shared/lower.isle
@@ -6,6 +6,20 @@
 
 ;;;; Rules for Control Flow ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
 
+;; Helper to place a conditional `Value` provided into a register. Pulley
+;; conditional values occupy the full low 32-bits of a register and so this
+;; needs to handle situations such as when the `Value` is 64-bits an explicit
+;; comparison must be made. Additionally if `Value` is smaller than 32-bits
+;; then it must be sign-extended up to at least 32 bits.
+(decl lower_cond (Value) XReg)
+(rule (lower_cond val @ (value_type $I64)) (pulley_xneq64 val (pulley_xconst8 0)))
+(rule (lower_cond val @ (value_type $I32)) val)
+(rule (lower_cond val @ (value_type $I16)) (pulley_zext16 val))
+(rule (lower_cond val @ (value_type $I8)) (pulley_zext8 val))
+
+;; Peel away explicit `uextend` values to take a look at the inner value.
+(rule 1 (lower_cond (uextend val)) (lower_cond val))
+
 ;; The main control-flow-lowering term: takes a control-flow instruction and
 ;; target(s) and emits the necessary instructions.
 (decl partial lower_branch (Inst MachLabelSlice) Unit)
@@ -15,8 +29,8 @@
       (emit_side_effect (pulley_jump label)))
 
 ;; Generic case for conditional branches.
-(rule -1 (lower_branch (brif (maybe_uextend c) _ _) (two_targets then else))
-      (emit_side_effect (pulley_br_if (zext c) then else)))
+(rule -1 (lower_branch (brif c _ _) (two_targets then else))
+      (emit_side_effect (pulley_br_if32 (lower_cond c) then else)))
 
 ;; Conditional branches on `icmp`s.
 (rule (lower_branch (brif (maybe_uextend (icmp cc a b @ (value_type $I32))) _ _)
@@ -420,8 +434,14 @@
 
 ;;;; Rules for `uextend` ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
 
-(rule (lower (has_type (fits_in_64 _) (uextend val)))
-  (zext val))
+(rule (lower (has_type (fits_in_64 _) (uextend val @ (value_type $I32))))
+  (pulley_zext32 val))
+
+(rule (lower (has_type (fits_in_64 _) (uextend val @ (value_type $I16))))
+  (pulley_zext16 val))
+
+(rule (lower (has_type (fits_in_64 _) (uextend val @ (value_type $I8))))
+  (pulley_zext8 val))
 
 ;;;; Rules for `sextend` ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
 
@@ -446,3 +466,17 @@
 
 (rule (lower (has_type $I64 (uadd_overflow_trap a b tc)))
   (pulley_xadd64_uoverflow_trap a b tc))
+
+;;;; Rules for `select` ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
+
+(rule 0 (lower (has_type (ty_int (fits_in_32 _)) (select c a b)))
+  (pulley_xselect32 (lower_cond c) a b))
+
+(rule 1 (lower (has_type $I64 (select c a b)))
+  (pulley_xselect64 (lower_cond c) a b))
+
+(rule 1 (lower (has_type $F32 (select c a b)))
+  (pulley_fselect32 (lower_cond c) a b))
+
+(rule 1 (lower (has_type $F64 (select c a b)))
+  (pulley_fselect64 (lower_cond c) a b))
diff --git a/cranelift/filetests/filetests/isa/pulley32/brif.clif b/cranelift/filetests/filetests/isa/pulley32/brif.clif
@@ -17,7 +17,7 @@ block2:
 ; VCode:
 ; block0:
 ;   zext8 x4, x0
-;   br_if x4, label2; jump label1
+;   br_if32 x4, label2; jump label1
 ; block1:
 ;   xconst8 x0, 0
 ;   ret
@@ -27,7 +27,7 @@ block2:
 ;
 ; Disassembled:
 ; zext8 x4, x0
-; br_if x4, 0xa    // target = 0xd
+; br_if32 x4, 0xa    // target = 0xd
 ; xconst8 x0, 0
 ; ret
 ; xconst8 x0, 1
@@ -49,7 +49,7 @@ block2:
 ; VCode:
 ; block0:
 ;   zext16 x4, x0
-;   br_if x4, label2; jump label1
+;   br_if32 x4, label2; jump label1
 ; block1:
 ;   xconst8 x0, 0
 ;   ret
@@ -59,7 +59,7 @@ block2:
 ;
 ; Disassembled:
 ; zext16 x4, x0
-; br_if x4, 0xa    // target = 0xd
+; br_if32 x4, 0xa    // target = 0xd
 ; xconst8 x0, 0
 ; ret
 ; xconst8 x0, 1
@@ -80,8 +80,7 @@ block2:
 
 ; VCode:
 ; block0:
-;   zext32 x4, x0
-;   br_if x4, label2; jump label1
+;   br_if32 x0, label2; jump label1
 ; block1:
 ;   xconst8 x0, 0
 ;   ret
@@ -90,8 +89,7 @@ block2:
 ;   ret
 ;
 ; Disassembled:
-; zext32 x4, x0
-; br_if x4, 0xa    // target = 0xd
+; br_if32 x0, 0xa    // target = 0xa
 ; xconst8 x0, 0
 ; ret
 ; xconst8 x0, 1
@@ -112,7 +110,9 @@ block2:
 
 ; VCode:
 ; block0:
-;   br_if x0, label2; jump label1
+;   xconst8 x4, 0
+;   xneq64 x6, x0, x4
+;   br_if32 x6, label2; jump label1
 ; block1:
 ;   xconst8 x0, 0
 ;   ret
@@ -121,7 +121,9 @@ block2:
 ;   ret
 ;
 ; Disassembled:
-; br_if x0, 0xa    // target = 0xa
+; xconst8 x4, 0
+; xneq64 x6, x0, x4
+; br_if32 x6, 0xa    // target = 0x10
 ; xconst8 x0, 0
 ; ret
 ; xconst8 x0, 1
@@ -145,7 +147,7 @@ block2:
 ; block0:
 ;   xeq32 x6, x0, x1
 ;   zext8 x6, x6
-;   br_if x6, label2; jump label1
+;   br_if32 x6, label2; jump label1
 ; block1:
 ;   xconst8 x0, 0
 ;   ret
@@ -156,7 +158,7 @@ block2:
 ; Disassembled:
 ; xeq32 x6, x0, x1
 ; zext8 x6, x6
-; br_if x6, 0xa    // target = 0x10
+; br_if32 x6, 0xa    // target = 0x10
 ; xconst8 x0, 0
 ; ret
 ; xconst8 x0, 1
@@ -180,7 +182,7 @@ block2:
 ; block0:
 ;   xneq32 x6, x0, x1
 ;   zext8 x6, x6
-;   br_if x6, label2; jump label1
+;   br_if32 x6, label2; jump label1
 ; block1:
 ;   xconst8 x0, 0
 ;   ret
@@ -191,7 +193,7 @@ block2:
 ; Disassembled:
 ; xneq32 x6, x0, x1
 ; zext8 x6, x6
-; br_if x6, 0xa    // target = 0x10
+; br_if32 x6, 0xa    // target = 0x10
 ; xconst8 x0, 0
 ; ret
 ; xconst8 x0, 1
@@ -246,7 +248,7 @@ block2:
 ; block0:
 ;   xulteq64 x6, x1, x0
 ;   zext8 x6, x6
-;   br_if x6, label2; jump label1
+;   br_if32 x6, label2; jump label1
 ; block1:
 ;   xconst8 x0, 0
 ;   ret
@@ -257,7 +259,7 @@ block2:
 ; Disassembled:
 ; xulteq64 x6, x1, x0
 ; zext8 x6, x6
-; br_if x6, 0xa    // target = 0x10
+; br_if32 x6, 0xa    // target = 0x10
 ; xconst8 x0, 0
 ; ret
 ; xconst8 x0, 1

diff --git a/cranelift/filetests/filetests/isa/pulley32/jump.clif b/cranelift/filetests/filetests/isa/pulley32/jump.clif
@@ -20,7 +20,7 @@ block3(v3: i8):
 ; VCode:
 ; block0:
 ;   zext8 x5, x0
-;   br_if x5, label2; jump label1
+;   br_if32 x5, label2; jump label1
 ; block1:
 ;   xconst8 x0, 0
 ;   jump label3
@@ -32,7 +32,7 @@ block3(v3: i8):
 ;
 ; Disassembled:
 ; zext8 x5, x0
-; br_if x5, 0xe    // target = 0x11
+; br_if32 x5, 0xe    // target = 0x11
 ; xconst8 x0, 0
 ; jump 0x8    // target = 0x14
 ; xconst8 x0, 1