Skip to content

Commit

Permalink
Fix inconsistencies with brz, brnz, and select on riscv64
Browse files Browse the repository at this point in the history
  • Loading branch information
elliottt committed Oct 17, 2022
1 parent 0c753ac commit 47118ea
Show file tree
Hide file tree
Showing 10 changed files with 117 additions and 68 deletions.
19 changes: 17 additions & 2 deletions cranelift/codegen/src/isa/riscv64/inst.isle
Original file line number Diff line number Diff line change
Expand Up @@ -1896,14 +1896,29 @@
(decl lower_brz_or_nz (IntCC ValueRegs VecMachLabel Type) InstOutput)
(extern constructor lower_brz_or_nz lower_brz_or_nz)

;; Normalize a value by masking to its bit-size.
(decl normalize_value (Type ValueRegs) ValueRegs)

(rule (normalize_value $I8 r)
(value_reg (alu_rr_imm12 (AluOPRRI.Andi) r (imm12_const 255))))
(rule (normalize_value $I16 r)
(value_reg (alu_rrr (AluOPRRR.And) r (imm $I16 65535))))
(rule (normalize_value $I32 r)
(value_reg (alu_rr_imm12 (AluOPRRI.Andi) r (imm12_const -1))))

(rule (normalize_value $I64 r) r)
(rule (normalize_value $I128 r) r)
(rule (normalize_value $F32 r) r)
(rule (normalize_value $F64 r) r)

;;;;;
(rule
(lower_branch (brz v @ (value_type ty) _ _) targets)
(lower_brz_or_nz (IntCC.Equal) v targets ty))
(lower_brz_or_nz (IntCC.Equal) (normalize_value ty v) targets ty))
;;;;
(rule
(lower_branch (brnz v @ (value_type ty) _ _) targets)
(lower_brz_or_nz (IntCC.NotEqual) v targets ty))
(lower_brz_or_nz (IntCC.NotEqual) (normalize_value ty v) targets ty))

;;;
(rule
Expand Down
3 changes: 1 addition & 2 deletions cranelift/codegen/src/isa/riscv64/inst/emit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1039,9 +1039,8 @@ impl MachInstEmit for Inst {
&Inst::CondBr {
taken,
not_taken,
kind,
mut kind,
} => {
let mut kind = kind;
kind.rs1 = allocs.next(kind.rs1);
kind.rs2 = allocs.next(kind.rs2);
match taken {
Expand Down
2 changes: 1 addition & 1 deletion cranelift/codegen/src/isa/riscv64/inst/unwind/systemv.rs
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ mod tests {

assert_eq!(
format!("{:?}", fde),
"FrameDescriptionEntry { address: Constant(4321), length: 12, lsda: None, instructions: [] }"
"FrameDescriptionEntry { address: Constant(4321), length: 16, lsda: None, instructions: [] }"
);
}

Expand Down
4 changes: 2 additions & 2 deletions cranelift/codegen/src/isa/riscv64/lower.isle
Original file line number Diff line number Diff line change
Expand Up @@ -601,8 +601,8 @@

;;;;; Rules for `select`;;;;;;;;;
(rule
(lower (has_type ty (select c x y)))
(gen_select ty c x y)
(lower (has_type ty (select c @ (value_type cty) x y)))
(gen_select ty (normalize_value cty c) x y)
)

;;;;; Rules for `bitselect`;;;;;;;;;
Expand Down
6 changes: 5 additions & 1 deletion cranelift/codegen/src/isa/riscv64/lower/isle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -270,7 +270,11 @@ impl generated_code::Context for IsleContext<'_, '_, MInst, Flags, IsaFlags, 6>
rd.to_reg()
}
fn imm12_const(&mut self, val: i32) -> Imm12 {
Imm12::maybe_from_u64(val as u64).unwrap()
if let Some(res) = Imm12::maybe_from_u64(val as u64) {
res
} else {
panic!("Unable to make an Imm12 value from {}", val)
}
}
fn imm12_const_add(&mut self, val: i32, add: i32) -> Imm12 {
Imm12::maybe_from_u64((val + add) as u64).unwrap()
Expand Down
50 changes: 30 additions & 20 deletions cranelift/filetests/filetests/isa/riscv64/condbr.clif
Original file line number Diff line number Diff line change
Expand Up @@ -209,8 +209,9 @@ block1:
}

; block0:
; eq a2,[a0,a1],[a2,a3]##ty=i128
; bne a2,zero,taken(label1),not_taken(label2)
; eq a3,[a0,a1],[a2,a3]##ty=i128
; andi a3,a3,255
; bne a3,zero,taken(label1),not_taken(label2)
; block1:
; j label3
; block2:
Expand All @@ -228,8 +229,9 @@ block1:
}

; block0:
; ne a2,[a0,a1],[a2,a3]##ty=i128
; bne a2,zero,taken(label1),not_taken(label2)
; ne a3,[a0,a1],[a2,a3]##ty=i128
; andi a3,a3,255
; bne a3,zero,taken(label1),not_taken(label2)
; block1:
; j label3
; block2:
Expand All @@ -247,8 +249,9 @@ block1:
}

; block0:
; slt a2,[a0,a1],[a2,a3]##ty=i128
; bne a2,zero,taken(label1),not_taken(label2)
; slt a3,[a0,a1],[a2,a3]##ty=i128
; andi a3,a3,255
; bne a3,zero,taken(label1),not_taken(label2)
; block1:
; j label3
; block2:
Expand All @@ -266,8 +269,9 @@ block1:
}

; block0:
; ult a2,[a0,a1],[a2,a3]##ty=i128
; bne a2,zero,taken(label1),not_taken(label2)
; ult a3,[a0,a1],[a2,a3]##ty=i128
; andi a3,a3,255
; bne a3,zero,taken(label1),not_taken(label2)
; block1:
; j label3
; block2:
Expand All @@ -285,8 +289,9 @@ block1:
}

; block0:
; sle a2,[a0,a1],[a2,a3]##ty=i128
; bne a2,zero,taken(label1),not_taken(label2)
; sle a3,[a0,a1],[a2,a3]##ty=i128
; andi a3,a3,255
; bne a3,zero,taken(label1),not_taken(label2)
; block1:
; j label3
; block2:
Expand All @@ -304,8 +309,9 @@ block1:
}

; block0:
; ule a2,[a0,a1],[a2,a3]##ty=i128
; bne a2,zero,taken(label1),not_taken(label2)
; ule a3,[a0,a1],[a2,a3]##ty=i128
; andi a3,a3,255
; bne a3,zero,taken(label1),not_taken(label2)
; block1:
; j label3
; block2:
Expand All @@ -323,8 +329,9 @@ block1:
}

; block0:
; sgt a2,[a0,a1],[a2,a3]##ty=i128
; bne a2,zero,taken(label1),not_taken(label2)
; sgt a3,[a0,a1],[a2,a3]##ty=i128
; andi a3,a3,255
; bne a3,zero,taken(label1),not_taken(label2)
; block1:
; j label3
; block2:
Expand All @@ -342,8 +349,9 @@ block1:
}

; block0:
; ugt a2,[a0,a1],[a2,a3]##ty=i128
; bne a2,zero,taken(label1),not_taken(label2)
; ugt a3,[a0,a1],[a2,a3]##ty=i128
; andi a3,a3,255
; bne a3,zero,taken(label1),not_taken(label2)
; block1:
; j label3
; block2:
Expand All @@ -361,8 +369,9 @@ block1:
}

; block0:
; sge a2,[a0,a1],[a2,a3]##ty=i128
; bne a2,zero,taken(label1),not_taken(label2)
; sge a3,[a0,a1],[a2,a3]##ty=i128
; andi a3,a3,255
; bne a3,zero,taken(label1),not_taken(label2)
; block1:
; j label3
; block2:
Expand All @@ -380,8 +389,9 @@ block1:
}

; block0:
; uge a2,[a0,a1],[a2,a3]##ty=i128
; bne a2,zero,taken(label1),not_taken(label2)
; uge a3,[a0,a1],[a2,a3]##ty=i128
; andi a3,a3,255
; bne a3,zero,taken(label1),not_taken(label2)
; block1:
; j label3
; block2:
Expand Down
17 changes: 10 additions & 7 deletions cranelift/filetests/filetests/isa/riscv64/condops.clif
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,8 @@ block0(v0: i8, v1: i8, v2: i8):
}

; block0:
; select_i8 a0,a1,a2##condition=a0
; andi a3,a0,255
; select_i8 a0,a1,a2##condition=a3
; ret

function %i(i32, i8, i8) -> i8 {
Expand All @@ -67,11 +68,12 @@ block0(v0: i32, v1: i8, v2: i8):
}

; block0:
; li a3,42
; uext.w a5,a0
; uext.w a7,a3
; eq t4,a5,a7##ty=i32
; select_i8 a0,a1,a2##condition=t4
; li a4,42
; uext.w a6,a0
; uext.w t3,a4
; eq t0,a6,t3##ty=i32
; andi a6,t0,255
; select_i8 a0,a1,a2##condition=a6
; ret

function %i128_select(i8, i128, i128) -> i128 {
Expand All @@ -81,6 +83,7 @@ block0(v0: i8, v1: i128, v2: i128):
}

; block0:
; select_i128 [a0,a1],[a1,a2],[a3,a4]##condition=a0
; andi a5,a0,255
; select_i128 [a0,a1],[a1,a2],[a3,a4]##condition=a5
; ret

38 changes: 20 additions & 18 deletions cranelift/filetests/filetests/isa/riscv64/heap-addr.clif
Original file line number Diff line number Diff line change
Expand Up @@ -13,16 +13,17 @@ block0(v0: i64, v1: i32):
}

; block0:
; uext.w t3,a1
; ld t4,0(a0)
; addi t4,t4,0
; ugt t0,t3,t4##ty=i64
; beq t0,zero,taken(label1),not_taken(label2)
; uext.w t4,a1
; ld t0,0(a0)
; addi t0,t0,0
; ugt t1,t4,t0##ty=i64
; andi t1,t1,255
; beq t1,zero,taken(label1),not_taken(label2)
; block1:
; add t0,a0,t3
; ugt t3,t3,t4##ty=i64
; li t1,0
; selectif_spectre_guard a0,t1,t0##test=t3
; add t1,a0,t4
; ugt t4,t4,t0##ty=i64
; li t2,0
; selectif_spectre_guard a0,t2,t1##test=t4
; ret
; block2:
; udf##trap_code=heap_oob
Expand All @@ -37,16 +38,17 @@ block0(v0: i64, v1: i32):
}

; block0:
; uext.w t3,a1
; lui a7,16
; ugt t4,t3,a7##ty=i64
; beq t4,zero,taken(label1),not_taken(label2)
; uext.w t4,a1
; lui t3,16
; ugt t0,t4,t3##ty=i64
; andi t0,t0,255
; beq t0,zero,taken(label1),not_taken(label2)
; block1:
; add t4,a0,t3
; lui a7,16
; ugt t0,t3,a7##ty=i64
; li t1,0
; selectif_spectre_guard a0,t1,t4##test=t0
; add t0,a0,t4
; lui t3,16
; ugt t1,t4,t3##ty=i64
; li t2,0
; selectif_spectre_guard a0,t2,t0##test=t1
; ret
; block2:
; udf##trap_code=heap_oob
Expand Down
31 changes: 16 additions & 15 deletions cranelift/filetests/filetests/isa/riscv64/reftypes.clif
Original file line number Diff line number Diff line change
Expand Up @@ -65,37 +65,38 @@ block3(v7: r64, v8: r64):
; sd ra,8(sp)
; sd fp,0(sp)
; mv fp,sp
; sd s9,-8(sp)
; sd s10,-8(sp)
; add sp,-48
; block0:
; sd a0,8(nominal_sp)
; sd a1,16(nominal_sp)
; mv s9,a2
; load_sym a3,%f+0
; callind a3
; load_addr a2,nsp+0
; ld t1,8(nominal_sp)
; sd t1,0(a2)
; beq a0,zero,taken(label1),not_taken(label3)
; mv s10,a2
; load_sym a4,%f+0
; callind a4
; load_addr a3,nsp+0
; ld t2,8(nominal_sp)
; sd t2,0(a3)
; andi a4,a0,255
; beq a4,zero,taken(label1),not_taken(label3)
; block1:
; j label2
; block2:
; mv a1,t1
; mv a1,t2
; ld a0,16(nominal_sp)
; j label5
; block3:
; j label4
; block4:
; mv a0,t1
; mv a0,t2
; ld a1,16(nominal_sp)
; j label5
; block5:
; load_addr a4,nsp+0
; ld a4,0(a4)
; mv a2,s9
; sd a4,0(a2)
; load_addr a5,nsp+0
; ld a5,0(a5)
; mv a2,s10
; sd a5,0(a2)
; add sp,+48
; ld s9,-8(sp)
; ld s10,-8(sp)
; ld ra,8(sp)
; ld fp,0(sp)
; add sp,+16
Expand Down
15 changes: 15 additions & 0 deletions cranelift/filetests/filetests/runtests/select.clif
Original file line number Diff line number Diff line change
Expand Up @@ -93,3 +93,18 @@ block0(v0: f32, v1: f32):
; run: %select_uno_f32(0x0.0, 0x42.42) == 0
; run: %select_uno_f32(0x0.0, NaN) == 1
; run: %select_uno_f32(-NaN, 0x42.42) == 1

function %select_overflow(i8) -> i8 {
block0(v0: i8):
v1 = iconst.i8 255
v2 = iadd v0, v1
v3 = iconst.i8 1
v4 = iconst.i8 0
v5 = select v2, v3, v4
return v5
}

; run: %select_overflow(0) == 1
; run: %select_overflow(2) == 1
; run: %select_overflow(1) == 0
; run: %select_overflow(98) == 1

0 comments on commit 47118ea

Please sign in to comment.