Skip to content

riscv64: Delete SelectIf instruction #5888

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
afonso360 merged 5 commits into from
Apr 11, 2023
Merged

riscv64: Delete SelectIf instruction #5888

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
497922f
riscv64: Delete `SelectIf` instruction
afonso360 Apr 7, 2023
67512d5
riscv64: Fix typo in comment
afonso360 Apr 10, 2023
85fca00
riscv64: Improve `bmask` codegen
afonso360 Apr 10, 2023
a2c4130
riscv64: Use `lower_bmask` in `select_spectre_guard`
afonso360 Apr 10, 2023
1fbe9c6
riscv64: Use `lower_bmask` to extend values in `select_spectre_guard`
afonso360 Apr 11, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 38 additions & 18 deletions cranelift/codegen/src/isa/riscv64/inst.isle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -257,12 +257,7 @@
(is_signed bool)
(in_type Type)
(out_type Type))
(SelectIf
(if_spectre_guard bool)
(rd VecWritableReg)
(test Reg)
(x ValueRegs)
(y ValueRegs))

(RawData (data VecU8))

;; An unwind pseudo-instruction.
Expand Down Expand Up @@ -872,6 +867,12 @@
(rule (rv_sltu rs1 rs2)
(alu_rrr (AluOPRRR.SltU) rs1 rs2))

;; Helper for emitting the `snez` instruction.
;; This instruction is a mnemonic for `sltu rd, zero, rs`.
(decl rv_snez (Reg) Reg)
(rule (rv_snez rs1)
(rv_sltu (zero_reg) rs1))

;; Helper for emiting the `sltiu` ("Set Less Than Immediate Unsigned") instruction.
;; rd ← rs1 < imm
(decl rv_sltiu (Reg Imm12) Reg)
Expand Down Expand Up @@ -1371,15 +1372,35 @@
(rule (select_addi (fits_in_64 ty)) (AluOPRRI.Addi))


(decl bnot_128 (ValueRegs) ValueRegs)
(rule
(bnot_128 val)
(let
(;; low part.
(low Reg (rv_not (value_regs_get val 0)))
;; high part.
(high Reg (rv_not (value_regs_get val 1))))
(value_regs low high)))
(decl gen_bnot (Type ValueRegs) ValueRegs)
(rule 1 (gen_bnot $I128 x)
(let ((lo Reg (rv_not (value_regs_get x 0)))
(hi Reg (rv_not (value_regs_get x 1))))
(value_regs lo hi)))

(rule 0 (gen_bnot (fits_in_64 _) x)
(rv_not x))


(decl gen_and (Type ValueRegs ValueRegs) ValueRegs)
(rule 1 (gen_and $I128 x y)
(value_regs
(rv_and (value_regs_get x 0) (value_regs_get y 0))
(rv_and (value_regs_get x 1) (value_regs_get y 1))))

(rule 0 (gen_and (fits_in_64 _) x y)
(rv_and (value_regs_get x 0) (value_regs_get y 0)))


(decl gen_or (Type ValueRegs ValueRegs) ValueRegs)
(rule 1 (gen_or $I128 x y)
(value_regs
(rv_or (value_regs_get x 0) (value_regs_get y 0))
(rv_or (value_regs_get x 1) (value_regs_get y 1))))

(rule 0 (gen_or (fits_in_64 _) x y)
(rv_or (value_regs_get x 0) (value_regs_get y 0)))


(decl lower_bit_reverse (Reg Type) Reg)

Expand Down Expand Up @@ -2511,9 +2532,8 @@
0
(lower_bmask (fits_in_64 _) (fits_in_64 in_ty) val)
(let ((input Reg (normalize_cmp_value in_ty val (ExtendOp.Zero)))
(zero Reg (zero_reg))
(ones Reg (load_imm12 -1)))
(value_reg (gen_select_reg (IntCC.Equal) zero input zero ones))))
(non_zero Reg (rv_snez input)))
(value_reg (rv_neg non_zero))))

;; Bitwise-or the two registers that make up the 128-bit value, then recurse as
;; though it was a 64-bit value.
Expand Down
42 changes: 0 additions & 42 deletions cranelift/codegen/src/isa/riscv64/inst/emit.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1864,48 +1864,6 @@ impl MachInstEmit for Inst {
}
sink.put_data(Inst::TRAP_OPCODE);
}
&Inst::SelectIf {
if_spectre_guard: _if_spectre_guard, // _if_spectre_guard not use because it is used to not be removed by optimization pass and some other staff.
ref rd,
test,
ref x,
ref y,
} => {
let label_select_x = sink.get_label();
let label_select_y = sink.get_label();
let label_jump_over = sink.get_label();
let test = allocs.next(test);
let x = alloc_value_regs(x, &mut allocs);
let y = alloc_value_regs(y, &mut allocs);
let rd: Vec<_> = rd.iter().map(|r| allocs.next_writable(*r)).collect();
Inst::CondBr {
taken: BranchTarget::Label(label_select_x),
not_taken: BranchTarget::Label(label_select_y),
kind: IntegerCompare {
kind: IntCC::NotEqual,
rs1: test,
rs2: zero_reg(),
},
}
.emit(&[], sink, emit_info, state);

// here select x.
sink.bind_label(label_select_x, &mut state.ctrl_plane);
gen_moves(&rd[..], x.regs())
.into_iter()
.for_each(|i| i.emit(&[], sink, emit_info, state));
// jump over
Inst::Jal {
dest: BranchTarget::Label(label_jump_over),
}
.emit(&[], sink, emit_info, state);
// here select y.
sink.bind_label(label_select_y, &mut state.ctrl_plane);
gen_moves(&rd[..], y.regs())
.into_iter()
.for_each(|i| i.emit(&[], sink, emit_info, state));
sink.bind_label(label_jump_over, &mut state.ctrl_plane);
}
&Inst::AtomicLoad { rd, ty, p } => {
let p = allocs.next(p);
let rd = allocs.next_writable(rd);
Expand Down
37 changes: 0 additions & 37 deletions cranelift/codegen/src/isa/riscv64/inst/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -526,18 +526,6 @@ fn riscv64_get_operands<F: Fn(VReg) -> VReg>(inst: &Inst, collector: &mut Operan
collector.reg_early_def(tmp);
collector.reg_early_def(rd);
}
&Inst::SelectIf {
ref rd,
test,
ref x,
ref y,
..
} => {
collector.reg_use(test);
collector.reg_uses(x.regs());
collector.reg_uses(y.regs());
rd.iter().for_each(|r| collector.reg_early_def(*r));
}
&Inst::RawData { .. } => {}
&Inst::AtomicStore { src, p, .. } => {
collector.reg_use(src);
Expand Down Expand Up @@ -1012,31 +1000,6 @@ impl Inst {
rd, rs, tmp, tmp2, step, ty
)
}
&Inst::SelectIf {
if_spectre_guard,
ref rd,
test,
ref x,
ref y,
} => {
let test = format_reg(test, allocs);
let x = format_regs(x.regs(), allocs);
let y = format_regs(y.regs(), allocs);
let rd: Vec<_> = rd.iter().map(|r| r.to_reg()).collect();
let rd = format_regs(&rd[..], allocs);
format!(
"selectif{} {},{},{}##test={}",
if if_spectre_guard {
"_spectre_guard"
} else {
""
},
rd,
x,
y,
test
)
}
&Inst::Popcnt {
sum,
step,
Expand Down
47 changes: 22 additions & 25 deletions cranelift/codegen/src/isa/riscv64/lower.isle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -230,8 +230,8 @@
(rv_remu x y)))

;;;; Rules for `and` ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
(rule -1 (lower (has_type (fits_in_64 (ty_int ty)) (band x y)))
(rv_and x y))
(rule -1 (lower (has_type (ty_int ty) (band x y)))
(gen_and ty x y))

;; Special cases for when one operand is an immediate that fits in 12 bits.
(rule 2 (lower (has_type (fits_in_64 (ty_int ty)) (band x (imm12_from_value y))))
Expand All @@ -240,9 +240,6 @@
(rule 1 (lower (has_type (fits_in_64 (ty_int ty)) (band (imm12_from_value x) y)))
(rv_andi y x))

(rule (lower (has_type $I128 (band x y)))
(lower_b128_binary (AluOPRRR.And) x y))

(rule (lower (has_type $F32 (band x y)))
(lower_float_binary (AluOPRRR.And) x y $F32))

Expand Down Expand Up @@ -277,8 +274,8 @@


;;;; Rules for `or` ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
(rule -1 (lower (has_type (fits_in_64 (ty_int ty)) (bor x y)))
(rv_or x y))
(rule -1 (lower (has_type (ty_int ty) (bor x y)))
(gen_or ty x y))

;; Special cases for when one operand is an immediate that fits in 12 bits.
(rule 2 (lower (has_type (fits_in_64 (ty_int ty)) (bor x (imm12_from_value y))))
Expand All @@ -287,9 +284,6 @@
(rule 1 (lower (has_type (fits_in_64 (ty_int ty)) (bor (imm12_from_value x) y)))
(rv_ori y x))

(rule (lower (has_type $I128 (bor x y)))
(lower_b128_binary (AluOPRRR.Or) x y))

(rule (lower (has_type $F32 (bor x y)))
(lower_float_binary (AluOPRRR.Or) x y $F32))

Expand Down Expand Up @@ -345,11 +339,8 @@


;;;; Rules for `bnot` ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
(rule -1 (lower (has_type (fits_in_64 (ty_int ty)) (bnot x)))
(rv_xori x (imm_from_neg_bits -1)))

(rule (lower (has_type $I128 (bnot x)))
(bnot_128 x))
(rule -1 (lower (has_type (ty_int ty) (bnot x)))
(gen_bnot ty x))

(rule
(lower (has_type $F32 (bnot x)))
Expand Down Expand Up @@ -927,17 +918,23 @@


;;;;; Rules for `select_spectre_guard`;;;;;;;;;
(rule
(lower (has_type r_ty (select_spectre_guard (icmp cc ca @ (value_type cty) cb) a b)))
(let
((dst VecWritableReg (alloc_vec_writable r_ty))
(r Reg (lower_icmp cc ca cb cty))
(_ Unit (emit (MInst.SelectIf $true (vec_writable_clone dst) r a b))))
(vec_writable_to_regs dst)))

(rule -1
(lower (has_type ty (select_spectre_guard c @ (value_type cty) x y)))
(gen_select ty (truthy_to_reg cty (normalize_cmp_value cty c (ExtendOp.Zero))) x y))
;; SelectSpectreGuard is equivalent to Select, but we should not use a branch based
;; lowering for it. Instead we use a conditional move based lowering.
;;
;; We don't have cmov's in RISC-V either, but we can emulate those using bitwise
;; operations, which is what we do below.
(rule (lower (has_type ty (select_spectre_guard cmp @ (value_type cmp_ty) x @ (value_type arg_ty) y)))
(let (;; Build a mask that is 0 or -1 depending on the input comparision value.
;; `lower_bmask` handles normalizing the input.
(mask ValueRegs (lower_bmask arg_ty cmp_ty cmp))
;; Using the mask above we can select either `x` or `y` by
;; performing a bitwise `and` on both sides and then merging them
;; together. We know that only the bits of one of the sides will be selected.
;; TODO: We can use `andn` here if we have `Zbb`
(lhs ValueRegs (gen_and arg_ty x mask))
(rhs ValueRegs (gen_and arg_ty y (gen_bnot arg_ty mask))))
(gen_or arg_ty lhs rhs)))

;;;;; Rules for `bmask`;;;;;;;;;
(rule
Expand Down
Loading