Remove workarounds for things unimplemented in Cranelift

bjorn3 · bjorn3 · commit 2e62516496b9 · 2021-07-28T17:30:39.000+02:00
Many are now implemented, so it is much nicer to directly use the
respective Cranelift instructions
diff --git a/src/base.rs b/src/base.rs
@@ -334,8 +334,6 @@ fn codegen_fn_content(fx: &mut FunctionCx<'_, '_, '_>) {
                         crate::optimize::peephole::maybe_unwrap_bool_not(&mut fx.bcx, discr);
                     let test_zero = if is_inverted { !test_zero } else { test_zero };
                     let discr = crate::optimize::peephole::maybe_unwrap_bint(&mut fx.bcx, discr);
-                    let discr =
-                        crate::optimize::peephole::make_branchable_value(&mut fx.bcx, discr);
                     if let Some(taken) = crate::optimize::peephole::maybe_known_branch_taken(
                         &fx.bcx, discr, test_zero,
                     ) {
diff --git a/src/cast.rs b/src/cast.rs
@@ -14,21 +14,6 @@ pub(crate) fn clif_intcast(
         (_, _) if from == to => val,
 
         // extend
-        (_, types::I128) => {
-            let lo = if from == types::I64 {
-                val
-            } else if signed {
-                fx.bcx.ins().sextend(types::I64, val)
-            } else {
-                fx.bcx.ins().uextend(types::I64, val)
-            };
-            let hi = if signed {
-                fx.bcx.ins().sshr_imm(lo, 63)
-            } else {
-                fx.bcx.ins().iconst(types::I64, 0)
-            };
-            fx.bcx.ins().iconcat(lo, hi)
-        }
         (_, _) if to.wider_or_equal(from) => {
             if signed {
                 fx.bcx.ins().sextend(to, val)
@@ -38,10 +23,6 @@ pub(crate) fn clif_intcast(
         }
 
         // reduce
-        (types::I128, _) => {
-            let (lsb, _msb) = fx.bcx.ins().isplit(val);
-            if to == types::I64 { lsb } else { fx.bcx.ins().ireduce(to, lsb) }
-        }
         (_, _) => fx.bcx.ins().ireduce(to, val),
     }
 }
diff --git a/src/debuginfo/emit.rs b/src/debuginfo/emit.rs
@@ -160,7 +160,7 @@ impl Writer for WriterRelocate {
                 let val = match eh_pe.application() {
                     gimli::DW_EH_PE_absptr => val,
                     gimli::DW_EH_PE_pcrel => {
-                        // TODO: better handling of sign
+                        // FIXME better handling of sign
                         let offset = self.len() as u64;
                         offset.wrapping_sub(val)
                     }
diff --git a/src/debuginfo/mod.rs b/src/debuginfo/mod.rs
@@ -46,7 +46,7 @@ impl<'tcx> DebugContext<'tcx> {
     pub(crate) fn new(tcx: TyCtxt<'tcx>, isa: &dyn TargetIsa) -> Self {
         let encoding = Encoding {
             format: Format::Dwarf32,
-            // TODO: this should be configurable
+            // FIXME this should be configurable
             // macOS doesn't seem to support DWARF > 3
             // 5 version is required for md5 file hash
             version: if tcx.sess.target.is_like_osx {
diff --git a/src/intrinsics/mod.rs b/src/intrinsics/mod.rs
@@ -611,9 +611,6 @@ pub(crate) fn codegen_intrinsic_call<'tcx>(
             let (val, has_overflow) = checked_res.load_scalar_pair(fx);
             let clif_ty = fx.clif_type(T).unwrap();
 
-            // `select.i8` is not implemented by Cranelift.
-            let has_overflow = fx.bcx.ins().uextend(types::I32, has_overflow);
-
             let (min, max) = type_min_max_value(&mut fx.bcx, clif_ty, signed);
 
             let val = match (intrinsic, signed) {
@@ -640,21 +637,11 @@ pub(crate) fn codegen_intrinsic_call<'tcx>(
         };
         rotate_left, <T>(v x, v y) {
             let layout = fx.layout_of(T);
-            let y = if fx.bcx.func.dfg.value_type(y) == types::I128 {
-                fx.bcx.ins().ireduce(types::I64, y)
-            } else {
-                y
-            };
             let res = fx.bcx.ins().rotl(x, y);
             ret.write_cvalue(fx, CValue::by_val(res, layout));
         };
         rotate_right, <T>(v x, v y) {
             let layout = fx.layout_of(T);
-            let y = if fx.bcx.func.dfg.value_type(y) == types::I128 {
-                fx.bcx.ins().ireduce(types::I64, y)
-            } else {
-                y
-            };
             let res = fx.bcx.ins().rotr(x, y);
             ret.write_cvalue(fx, CValue::by_val(res, layout));
         };
@@ -692,35 +679,13 @@ pub(crate) fn codegen_intrinsic_call<'tcx>(
         };
         ctlz | ctlz_nonzero, <T> (v arg) {
             // FIXME trap on `ctlz_nonzero` with zero arg.
-            let res = if T == fx.tcx.types.u128 || T == fx.tcx.types.i128 {
-                // FIXME verify this algorithm is correct
-                let (lsb, msb) = fx.bcx.ins().isplit(arg);
-                let lsb_lz = fx.bcx.ins().clz(lsb);
-                let msb_lz = fx.bcx.ins().clz(msb);
-                let msb_is_zero = fx.bcx.ins().icmp_imm(IntCC::Equal, msb, 0);
-                let lsb_lz_plus_64 = fx.bcx.ins().iadd_imm(lsb_lz, 64);
-                let res = fx.bcx.ins().select(msb_is_zero, lsb_lz_plus_64, msb_lz);
-                fx.bcx.ins().uextend(types::I128, res)
-            } else {
-                fx.bcx.ins().clz(arg)
-            };
+            let res = fx.bcx.ins().clz(arg);
             let res = CValue::by_val(res, fx.layout_of(T));
             ret.write_cvalue(fx, res);
         };
         cttz | cttz_nonzero, <T> (v arg) {
             // FIXME trap on `cttz_nonzero` with zero arg.
-            let res = if T == fx.tcx.types.u128 || T == fx.tcx.types.i128 {
-                // FIXME verify this algorithm is correct
-                let (lsb, msb) = fx.bcx.ins().isplit(arg);
-                let lsb_tz = fx.bcx.ins().ctz(lsb);
-                let msb_tz = fx.bcx.ins().ctz(msb);
-                let lsb_is_zero = fx.bcx.ins().icmp_imm(IntCC::Equal, lsb, 0);
-                let msb_tz_plus_64 = fx.bcx.ins().iadd_imm(msb_tz, 64);
-                let res = fx.bcx.ins().select(lsb_is_zero, msb_tz_plus_64, lsb_tz);
-                fx.bcx.ins().uextend(types::I128, res)
-            } else {
-                fx.bcx.ins().ctz(arg)
-            };
+            let res = fx.bcx.ins().ctz(arg);
             let res = CValue::by_val(res, fx.layout_of(T));
             ret.write_cvalue(fx, res);
         };
diff --git a/src/num.rs b/src/num.rs
@@ -67,19 +67,6 @@ pub(crate) fn codegen_binop<'tcx>(
                     let lhs = in_lhs.load_scalar(fx);
                     let rhs = in_rhs.load_scalar(fx);
 
-                    let (lhs, rhs) = if (bin_op == BinOp::Eq || bin_op == BinOp::Ne)
-                        && (in_lhs.layout().ty.kind() == fx.tcx.types.i8.kind()
-                            || in_lhs.layout().ty.kind() == fx.tcx.types.i16.kind())
-                    {
-                        // FIXME(CraneStation/cranelift#896) icmp_imm.i8/i16 with eq/ne for signed ints is implemented wrong.
-                        (
-                            fx.bcx.ins().sextend(types::I32, lhs),
-                            fx.bcx.ins().sextend(types::I32, rhs),
-                        )
-                    } else {
-                        (lhs, rhs)
-                    };
-
                     return codegen_compare_bin_op(fx, bin_op, signed, lhs, rhs);
                 }
                 _ => {}
diff --git a/src/optimize/peephole.rs b/src/optimize/peephole.rs
@@ -1,8 +1,6 @@
 //! Peephole optimizations that can be performed while creating clif ir.
 
-use cranelift_codegen::ir::{
-    condcodes::IntCC, types, InstBuilder, InstructionData, Opcode, Value, ValueDef,
-};
+use cranelift_codegen::ir::{condcodes::IntCC, InstructionData, Opcode, Value, ValueDef};
 use cranelift_frontend::FunctionBuilder;
 
 /// If the given value was produced by a `bint` instruction, return it's input, otherwise return the
@@ -37,43 +35,6 @@ pub(crate) fn maybe_unwrap_bool_not(bcx: &mut FunctionBuilder<'_>, arg: Value) -
     }
 }
 
-pub(crate) fn make_branchable_value(bcx: &mut FunctionBuilder<'_>, arg: Value) -> Value {
-    if bcx.func.dfg.value_type(arg).is_bool() {
-        return arg;
-    }
-
-    (|| {
-        let arg_inst = if let ValueDef::Result(arg_inst, 0) = bcx.func.dfg.value_def(arg) {
-            arg_inst
-        } else {
-            return None;
-        };
-
-        match bcx.func.dfg[arg_inst] {
-            // This is the lowering of Rvalue::Not
-            InstructionData::Load { opcode: Opcode::Load, arg: ptr, flags, offset } => {
-                // Using `load.i8 + uextend.i32` would legalize to `uload8 + ireduce.i8 +
-                // uextend.i32`. Just `uload8` is much faster.
-                match bcx.func.dfg.ctrl_typevar(arg_inst) {
-                    types::I8 => Some(bcx.ins().uload8(types::I32, flags, ptr, offset)),
-                    types::I16 => Some(bcx.ins().uload16(types::I32, flags, ptr, offset)),
-                    _ => None,
-                }
-            }
-            _ => None,
-        }
-    })()
-    .unwrap_or_else(|| {
-        match bcx.func.dfg.value_type(arg) {
-            types::I8 | types::I16 => {
-                // WORKAROUND for brz.i8 and brnz.i8 not yet being implemented
-                bcx.ins().uextend(types::I32, arg)
-            }
-            _ => arg,
-        }
-    })
-}
-
 /// Returns whether the branch is statically known to be taken or `None` if it isn't statically known.
 pub(crate) fn maybe_known_branch_taken(
     bcx: &FunctionBuilder<'_>,

Original file line number	Diff line number	Diff line change
`@@ -160,7 +160,7 @@ impl Writer for WriterRelocate {`
`160`	`160`	`let val = match eh_pe.application() {`
`161`	`161`	`gimli::DW_EH_PE_absptr => val,`
`162`	`162`	`gimli::DW_EH_PE_pcrel => {`
`163`		`- // TODO: better handling of sign`
	`163`	`+ // FIXME better handling of sign`
`164`	`164`	`let offset = self.len() as u64;`
`165`	`165`	`offset.wrapping_sub(val)`
`166`	`166`	`}`