From 86e77953f80cbc455b7dcb10bdd10dd3eb6a6e2e Mon Sep 17 00:00:00 2001 From: Chris Fallin Date: Fri, 21 Oct 2022 10:24:48 -0700 Subject: [PATCH] Fix some egraph-related issues. (#5088) This fixes #5086 by addressing two separate issues: - The `ValueDataPacked::set_type()` helper had an embarrassing bitfield-manipulation bug that would mangle the rest of a `ValueDef` when setting its type. This is not normally used, only when the egraph elaboration fills in types after-the-fact on a multi-value node. - The lowering rules for `isplit` on aarch64 and s390x were dispatching on the first output type, rather than the input type. When only the second output is used (as in the example in #5086), the first output type actually remains `INVALID` (and this is fine because it's never used). --- cranelift/codegen/src/ir/dfg.rs | 2 +- cranelift/codegen/src/isa/aarch64/lower.isle | 2 +- cranelift/codegen/src/isa/s390x/lower.isle | 2 +- .../filetests/filetests/egraph/isplit.clif | 18 ++++++++++++++++++ 4 files changed, 21 insertions(+), 3 deletions(-) create mode 100644 cranelift/filetests/filetests/egraph/isplit.clif diff --git a/cranelift/codegen/src/ir/dfg.rs b/cranelift/codegen/src/ir/dfg.rs index 26497ab679..f76c7a3517 100644 --- a/cranelift/codegen/src/ir/dfg.rs +++ b/cranelift/codegen/src/ir/dfg.rs @@ -523,7 +523,7 @@ impl ValueDataPacked { #[inline(always)] fn set_type(&mut self, ty: Type) { - self.0 &= !((1 << Self::TYPE_BITS) - 1) << Self::TYPE_SHIFT; + self.0 &= !(((1 << Self::TYPE_BITS) - 1) << Self::TYPE_SHIFT); self.0 |= (ty.repr() as u64) << Self::TYPE_SHIFT; } } diff --git a/cranelift/codegen/src/isa/aarch64/lower.isle b/cranelift/codegen/src/isa/aarch64/lower.isle index 89df5e822c..b26189add6 100644 --- a/cranelift/codegen/src/isa/aarch64/lower.isle +++ b/cranelift/codegen/src/isa/aarch64/lower.isle @@ -114,7 +114,7 @@ ;;;; Rules for `isplit` ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; -(rule (lower (has_type $I64 (isplit x))) +(rule (lower (isplit x @ (value_type $I128))) (let ((x_regs ValueRegs x) (x_lo ValueRegs (value_regs_get x_regs 0)) diff --git a/cranelift/codegen/src/isa/s390x/lower.isle b/cranelift/codegen/src/isa/s390x/lower.isle index c66c7dec69..965d17902e 100644 --- a/cranelift/codegen/src/isa/s390x/lower.isle +++ b/cranelift/codegen/src/isa/s390x/lower.isle @@ -60,7 +60,7 @@ ;;;; Rules for `isplit` ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; -(rule (lower (has_type (gpr64_ty ty) (isplit x))) +(rule (lower (isplit x @ (value_type $I128))) (let ((x_reg Reg x) (x_hi Reg (vec_extract_lane $I64X2 x_reg 0 (zero_reg))) (x_lo Reg (vec_extract_lane $I64X2 x_reg 1 (zero_reg)))) diff --git a/cranelift/filetests/filetests/egraph/isplit.clif b/cranelift/filetests/filetests/egraph/isplit.clif new file mode 100644 index 0000000000..ef3d8513a8 --- /dev/null +++ b/cranelift/filetests/filetests/egraph/isplit.clif @@ -0,0 +1,18 @@ +test interpret +test run +set opt_level=speed_and_size +set use_egraphs=true +set enable_llvm_abi_extensions=true +target x86_64 +target aarch64 +target s390x + +function %a(i128) -> i32 { +block0(v0: i128): + v1 = iconst.i32 -1 + v2, v3 = isplit v0 + v4 = ushr v1, v3 + return v4 +} + +; run: %a(871558149430564685057836279141) == 2147483647