From c77bec4dcb33973a386d924474d95bab6b23e263 Mon Sep 17 00:00:00 2001 From: Nick Fitzgerald Date: Mon, 1 Aug 2022 16:43:42 -0700 Subject: [PATCH] Cranelift: don't `emit` inside lowering rules for aarch64 (#4572) * Cranelift: Don't `emit` inside lowering rules in aarch64 The lowering rules should be "pure" and side-effect free, using helpers defined in `inst.isle` to perform actual side effects like emitting instructions. * Cranelift: use 80 width for section separators in aarch64 lowering rules --- cranelift/codegen/src/isa/aarch64/inst.isle | 10 ++++++++++ cranelift/codegen/src/isa/aarch64/lower.isle | 14 ++++++-------- 2 files changed, 16 insertions(+), 8 deletions(-) diff --git a/cranelift/codegen/src/isa/aarch64/inst.isle b/cranelift/codegen/src/isa/aarch64/inst.isle index 8b95b2d6df..58f5c0ea3e 100644 --- a/cranelift/codegen/src/isa/aarch64/inst.isle +++ b/cranelift/codegen/src/isa/aarch64/inst.isle @@ -1905,6 +1905,16 @@ (decl uqxtn2 (Reg Reg ScalarSize) Reg) (rule (uqxtn2 x y size) (vec_rr_narrow_high (VecRRNarrowOp.Uqxtn) x y size)) +;; Helper for generating `fence` instructions. +(decl aarch64_fence () SideEffectNoResult) +(rule (aarch64_fence) + (SideEffectNoResult.Inst (MInst.Fence))) + +;; Helper for generating `brk` instructions. +(decl brk () SideEffectNoResult) +(rule (brk) + (SideEffectNoResult.Inst (MInst.Brk))) + ;; Helper for generating `addp` instructions. (decl addp (Reg Reg VectorSize) Reg) (rule (addp x y size) (vec_rrr (VecALUOp.Addp) x y size)) diff --git a/cranelift/codegen/src/isa/aarch64/lower.isle b/cranelift/codegen/src/isa/aarch64/lower.isle index 1a79d908ae..ce536c1198 100644 --- a/cranelift/codegen/src/isa/aarch64/lower.isle +++ b/cranelift/codegen/src/isa/aarch64/lower.isle @@ -1700,28 +1700,26 @@ (result Reg (uqxtn2 low_half y (lane_size ty)))) result)) -;;;; Rules for `Fence` ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; +;;;; Rules for `Fence` ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; (rule (lower (fence)) - (let ((_ Unit (emit (MInst.Fence)))) - (output_none))) + (side_effect (aarch64_fence))) -;;;; Rules for `IsNull` ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; +;;;; Rules for `IsNull` ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; (rule (lower (has_type out_ty (is_null x @ (value_type ty)))) (with_flags (cmp_imm (operand_size ty) x (u8_into_imm12 0)) (materialize_bool_result (ty_bits out_ty) (Cond.Eq)))) -;;;; Rules for `IsInvalid` ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; +;;;; Rules for `IsInvalid` ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; (rule (lower (has_type out_ty (is_invalid x @ (value_type ty)))) (with_flags (cmn_imm (operand_size ty) x (u8_into_imm12 1)) (materialize_bool_result (ty_bits out_ty) (Cond.Eq)))) -;;;; Rules for `Debugtrap` ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; +;;;; Rules for `Debugtrap` ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; (rule (lower (debugtrap)) - (let ((_ Unit (emit (MInst.Brk)))) - (output_none))) + (side_effect (brk)))