Merge pull request rust-lang#1366 from bjorn3/rip_out_simd_ssa

bjorn3 · web-flow · commit ae60cb1287c8 · 2023-03-26T12:55:15.000+02:00
Don't store vector types in ssa variables
diff --git a/example/std_example.rs b/example/std_example.rs
@@ -1,4 +1,4 @@
-#![feature(core_intrinsics, generators, generator_trait, is_sorted)]
+#![feature(core_intrinsics, generators, generator_trait, is_sorted, repr_simd)]
 
 #[cfg(target_arch = "x86_64")]
 use std::arch::x86_64::*;
@@ -153,12 +153,20 @@ fn main() {
 
         enum Never {}
     }
+
+    foo(I64X2(0, 0));
 }
 
 fn panic(_: u128) {
     panic!();
 }
 
+#[repr(simd)]
+struct I64X2(i64, i64);
+
+#[allow(improper_ctypes_definitions)]
+extern "C" fn foo(_a: I64X2) {}
+
 #[cfg(target_arch = "x86_64")]
 #[target_feature(enable = "sse2")]
 unsafe fn test_simd() {
diff --git a/scripts/test_rustc_tests.sh b/scripts/test_rustc_tests.sh
@@ -112,7 +112,6 @@ rm tests/incremental/spike-neg2.rs # same
 
 rm tests/ui/simd/intrinsic/generic-reduction-pass.rs # simd_reduce_add_unordered doesn't accept an accumulator for integer vectors
 
-rm tests/ui/simd/intrinsic/generic-as.rs # crash when accessing vector type field (#1318)
 rm tests/ui/simd/simd-bitmask.rs # crash
 
 # bugs in the test suite
diff --git a/src/abi/comments.rs b/src/abi/comments.rs
@@ -100,7 +100,6 @@ pub(super) fn add_local_place_comments<'tcx>(
             assert_eq!(local, place_local);
             ("ssa", Cow::Owned(format!("var=({}, {})", var1.index(), var2.index())))
         }
-        CPlaceInner::VarLane(_local, _var, _lane) => unreachable!(),
         CPlaceInner::Addr(ptr, meta) => {
             let meta = if let Some(meta) = meta {
                 Cow::Owned(format!("meta={}", meta))
diff --git a/src/abi/pass_mode.rs b/src/abi/pass_mode.rs
@@ -84,7 +84,7 @@ impl<'tcx> ArgAbiExt<'tcx> for ArgAbi<'tcx, Ty<'tcx>> {
                     attrs
                 )],
                 Abi::Vector { .. } => {
-                    let vector_ty = crate::intrinsics::clif_vector_type(tcx, self.layout).unwrap();
+                    let vector_ty = crate::intrinsics::clif_vector_type(tcx, self.layout);
                     smallvec![AbiParam::new(vector_ty)]
                 }
                 _ => unreachable!("{:?}", self.layout.abi),
@@ -135,7 +135,7 @@ impl<'tcx> ArgAbiExt<'tcx> for ArgAbi<'tcx, Ty<'tcx>> {
                     (None, vec![AbiParam::new(scalar_to_clif_type(tcx, scalar))])
                 }
                 Abi::Vector { .. } => {
-                    let vector_ty = crate::intrinsics::clif_vector_type(tcx, self.layout).unwrap();
+                    let vector_ty = crate::intrinsics::clif_vector_type(tcx, self.layout);
                     (None, vec![AbiParam::new(vector_ty)])
                 }
                 _ => unreachable!("{:?}", self.layout.abi),
diff --git a/src/common.rs b/src/common.rs
@@ -72,19 +72,6 @@ fn clif_type_from_ty<'tcx>(tcx: TyCtxt<'tcx>, ty: Ty<'tcx>) -> Option<types::Typ
                 pointer_ty(tcx)
             }
         }
-        ty::Adt(adt_def, _) if adt_def.repr().simd() => {
-            let (element, count) = match &tcx.layout_of(ParamEnv::reveal_all().and(ty)).unwrap().abi
-            {
-                Abi::Vector { element, count } => (*element, *count),
-                _ => unreachable!(),
-            };
-
-            match scalar_to_clif_type(tcx, element).by(u32::try_from(count).unwrap()) {
-                // Cranelift currently only implements icmp for 128bit vectors.
-                Some(vector_ty) if vector_ty.bits() == 128 => vector_ty,
-                _ => return None,
-            }
-        }
         ty::Param(_) => bug!("ty param {:?}", ty),
         _ => return None,
     })
@@ -96,12 +83,7 @@ fn clif_pair_type_from_ty<'tcx>(
 ) -> Option<(types::Type, types::Type)> {
     Some(match ty.kind() {
         ty::Tuple(types) if types.len() == 2 => {
-            let a = clif_type_from_ty(tcx, types[0])?;
-            let b = clif_type_from_ty(tcx, types[1])?;
-            if a.is_vector() || b.is_vector() {
-                return None;
-            }
-            (a, b)
+            (clif_type_from_ty(tcx, types[0])?, clif_type_from_ty(tcx, types[1])?)
         }
         ty::RawPtr(TypeAndMut { ty: pointee_ty, mutbl: _ }) | ty::Ref(_, pointee_ty, _) => {
             if has_ptr_meta(tcx, *pointee_ty) {
diff --git a/src/intrinsics/mod.rs b/src/intrinsics/mod.rs
@@ -51,17 +51,13 @@ fn report_atomic_type_validation_error<'tcx>(
     fx.bcx.ins().trap(TrapCode::UnreachableCodeReached);
 }
 
-pub(crate) fn clif_vector_type<'tcx>(tcx: TyCtxt<'tcx>, layout: TyAndLayout<'tcx>) -> Option<Type> {
+pub(crate) fn clif_vector_type<'tcx>(tcx: TyCtxt<'tcx>, layout: TyAndLayout<'tcx>) -> Type {
     let (element, count) = match layout.abi {
         Abi::Vector { element, count } => (element, count),
         _ => unreachable!(),
     };
 
-    match scalar_to_clif_type(tcx, element).by(u32::try_from(count).unwrap()) {
-        // Cranelift currently only implements icmp for 128bit vectors.
-        Some(vector_ty) if vector_ty.bits() == 128 => Some(vector_ty),
-        _ => None,
-    }
+    scalar_to_clif_type(tcx, element).by(u32::try_from(count).unwrap()).unwrap()
 }
 
 fn simd_for_each_lane<'tcx>(
diff --git a/src/intrinsics/simd.rs b/src/intrinsics/simd.rs
@@ -253,7 +253,7 @@ pub(super) fn codegen_simd_intrinsic_call<'tcx>(
             }
 
             ret.write_cvalue(fx, base);
-            let ret_lane = ret.place_field(fx, mir::Field::new(idx.try_into().unwrap()));
+            let ret_lane = ret.place_lane(fx, idx.try_into().unwrap());
             ret_lane.write_cvalue(fx, val);
         }
 
diff --git a/src/value_and_place.rs b/src/value_and_place.rs
@@ -3,7 +3,6 @@
 use crate::prelude::*;
 
 use cranelift_codegen::ir::immediates::Offset32;
-use cranelift_codegen::ir::{InstructionData, Opcode};
 
 fn codegen_field<'tcx>(
     fx: &mut FunctionCx<'_, '_, 'tcx>,
@@ -214,17 +213,7 @@ impl<'tcx> CValue<'tcx> {
     ) -> CValue<'tcx> {
         let layout = self.1;
         match self.0 {
-            CValueInner::ByVal(val) => match layout.abi {
-                Abi::Vector { element: _, count } => {
-                    let count = u8::try_from(count).expect("SIMD type with more than 255 lanes???");
-                    let field = u8::try_from(field.index()).unwrap();
-                    assert!(field < count);
-                    let lane = fx.bcx.ins().extractlane(val, field);
-                    let field_layout = layout.field(&*fx, usize::from(field));
-                    CValue::by_val(lane, field_layout)
-                }
-                _ => unreachable!("value_field for ByVal with abi {:?}", layout.abi),
-            },
+            CValueInner::ByVal(_) => unreachable!(),
             CValueInner::ByValPair(val1, val2) => match layout.abi {
                 Abi::ScalarPair(_, _) => {
                     let val = match field.as_u32() {
@@ -258,16 +247,7 @@ impl<'tcx> CValue<'tcx> {
         let lane_layout = fx.layout_of(lane_ty);
         assert!(lane_idx < lane_count);
         match self.0 {
-            CValueInner::ByVal(val) => match layout.abi {
-                Abi::Vector { element: _, count: _ } => {
-                    assert!(lane_count <= u8::MAX.into(), "SIMD type with more than 255 lanes???");
-                    let lane_idx = u8::try_from(lane_idx).unwrap();
-                    let lane = fx.bcx.ins().extractlane(val, lane_idx);
-                    CValue::by_val(lane, lane_layout)
-                }
-                _ => unreachable!("value_lane for ByVal with abi {:?}", layout.abi),
-            },
-            CValueInner::ByValPair(_, _) => unreachable!(),
+            CValueInner::ByVal(_) | CValueInner::ByValPair(_, _) => unreachable!(),
             CValueInner::ByRef(ptr, None) => {
                 let field_offset = lane_layout.size * lane_idx;
                 let field_ptr = ptr.offset_i64(fx, i64::try_from(field_offset.bytes()).unwrap());
@@ -348,7 +328,6 @@ pub(crate) struct CPlace<'tcx> {
 pub(crate) enum CPlaceInner {
     Var(Local, Variable),
     VarPair(Local, Variable, Variable),
-    VarLane(Local, Variable, u8),
     Addr(Pointer, Option<Value>),
 }
 
@@ -442,12 +421,6 @@ impl<'tcx> CPlace<'tcx> {
                 //fx.bcx.set_val_label(val2, cranelift_codegen::ir::ValueLabel::new(var2.index()));
                 CValue::by_val_pair(val1, val2, layout)
             }
-            CPlaceInner::VarLane(_local, var, lane) => {
-                let val = fx.bcx.use_var(var);
-                //fx.bcx.set_val_label(val, cranelift_codegen::ir::ValueLabel::new(var.index()));
-                let val = fx.bcx.ins().extractlane(val, lane);
-                CValue::by_val(val, layout)
-            }
             CPlaceInner::Addr(ptr, extra) => {
                 if let Some(extra) = extra {
                     CValue::by_ref_unsized(ptr, extra, layout)
@@ -470,9 +443,9 @@ impl<'tcx> CPlace<'tcx> {
     pub(crate) fn to_ptr_maybe_unsized(self) -> (Pointer, Option<Value>) {
         match self.inner {
             CPlaceInner::Addr(ptr, extra) => (ptr, extra),
-            CPlaceInner::Var(_, _)
-            | CPlaceInner::VarPair(_, _, _)
-            | CPlaceInner::VarLane(_, _, _) => bug!("Expected CPlace::Addr, found {:?}", self),
+            CPlaceInner::Var(_, _) | CPlaceInner::VarPair(_, _, _) => {
+                bug!("Expected CPlace::Addr, found {:?}", self)
+            }
         }
     }
 
@@ -565,26 +538,6 @@ impl<'tcx> CPlace<'tcx> {
         let dst_layout = self.layout();
         let to_ptr = match self.inner {
             CPlaceInner::Var(_local, var) => {
-                if let ty::Array(element, len) = dst_layout.ty.kind() {
-                    // Can only happen for vector types
-                    let len = u32::try_from(len.eval_target_usize(fx.tcx, ParamEnv::reveal_all()))
-                        .unwrap();
-                    let vector_ty = fx.clif_type(*element).unwrap().by(len).unwrap();
-
-                    let data = match from.0 {
-                        CValueInner::ByRef(ptr, None) => {
-                            let mut flags = MemFlags::new();
-                            flags.set_notrap();
-                            ptr.load(fx, vector_ty, flags)
-                        }
-                        CValueInner::ByVal(_)
-                        | CValueInner::ByValPair(_, _)
-                        | CValueInner::ByRef(_, Some(_)) => bug!("array should be ByRef"),
-                    };
-
-                    fx.bcx.def_var(var, data);
-                    return;
-                }
                 let data = CValue(from.0, dst_layout).load_scalar(fx);
                 let dst_ty = fx.clif_type(self.layout().ty).unwrap();
                 transmute_value(fx, var, data, dst_ty);
@@ -603,22 +556,6 @@ impl<'tcx> CPlace<'tcx> {
                 transmute_value(fx, var2, data2, dst_ty2);
                 return;
             }
-            CPlaceInner::VarLane(_local, var, lane) => {
-                let data = from.load_scalar(fx);
-
-                // First get the old vector
-                let vector = fx.bcx.use_var(var);
-                //fx.bcx.set_val_label(vector, cranelift_codegen::ir::ValueLabel::new(var.index()));
-
-                // Next insert the written lane into the vector
-                let vector = fx.bcx.ins().insertlane(vector, data, lane);
-
-                // Finally write the new vector
-                //fx.bcx.set_val_label(vector, cranelift_codegen::ir::ValueLabel::new(var.index()));
-                fx.bcx.def_var(var, vector);
-
-                return;
-            }
             CPlaceInner::Addr(ptr, None) => {
                 if dst_layout.size == Size::ZERO || dst_layout.abi == Abi::Uninhabited {
                     return;
@@ -631,7 +568,6 @@ impl<'tcx> CPlace<'tcx> {
         let mut flags = MemFlags::new();
         flags.set_notrap();
         match from.layout().abi {
-            // FIXME make Abi::Vector work too
             Abi::Scalar(_) => {
                 let val = from.load_scalar(fx);
                 to_ptr.store(fx, val, flags);
@@ -692,39 +628,6 @@ impl<'tcx> CPlace<'tcx> {
         let layout = self.layout();
 
         match self.inner {
-            CPlaceInner::Var(local, var) => match layout.ty.kind() {
-                ty::Array(_, _) => {
-                    // Can only happen for vector types
-                    return CPlace {
-                        inner: CPlaceInner::VarLane(local, var, field.as_u32().try_into().unwrap()),
-                        layout: layout.field(fx, field.as_u32().try_into().unwrap()),
-                    };
-                }
-                ty::Adt(adt_def, substs) if layout.ty.is_simd() => {
-                    let f0_ty = adt_def.non_enum_variant().fields[0].ty(fx.tcx, substs);
-
-                    match f0_ty.kind() {
-                        ty::Array(_, _) => {
-                            assert_eq!(field.as_u32(), 0);
-                            return CPlace {
-                                inner: CPlaceInner::Var(local, var),
-                                layout: layout.field(fx, field.as_u32().try_into().unwrap()),
-                            };
-                        }
-                        _ => {
-                            return CPlace {
-                                inner: CPlaceInner::VarLane(
-                                    local,
-                                    var,
-                                    field.as_u32().try_into().unwrap(),
-                                ),
-                                layout: layout.field(fx, field.as_u32().try_into().unwrap()),
-                            };
-                        }
-                    }
-                }
-                _ => {}
-            },
             CPlaceInner::VarPair(local, var1, var2) => {
                 let layout = layout.field(&*fx, field.index());
 
@@ -766,15 +669,8 @@ impl<'tcx> CPlace<'tcx> {
         assert!(lane_idx < lane_count);
 
         match self.inner {
-            CPlaceInner::Var(local, var) => {
-                assert!(matches!(layout.abi, Abi::Vector { .. }));
-                CPlace {
-                    inner: CPlaceInner::VarLane(local, var, lane_idx.try_into().unwrap()),
-                    layout: lane_layout,
-                }
-            }
+            CPlaceInner::Var(_, _) => unreachable!(),
             CPlaceInner::VarPair(_, _, _) => unreachable!(),
-            CPlaceInner::VarLane(_, _, _) => unreachable!(),
             CPlaceInner::Addr(ptr, None) => {
                 let field_offset = lane_layout.size * lane_idx;
                 let field_ptr = ptr.offset_i64(fx, i64::try_from(field_offset.bytes()).unwrap());
@@ -793,32 +689,11 @@ impl<'tcx> CPlace<'tcx> {
             ty::Array(elem_ty, _) => {
                 let elem_layout = fx.layout_of(*elem_ty);
                 match self.inner {
-                    CPlaceInner::Var(local, var) => {
-                        // This is a hack to handle `vector_val.0[1]`. It doesn't allow dynamic
-                        // indexing.
-                        let lane_idx = match fx.bcx.func.dfg.insts
-                            [fx.bcx.func.dfg.value_def(index).unwrap_inst()]
-                        {
-                            InstructionData::UnaryImm { opcode: Opcode::Iconst, imm } => imm,
-                            _ => bug!(
-                                "Dynamic indexing into a vector type is not supported: {self:?}[{index}]"
-                            ),
-                        };
-                        return CPlace {
-                            inner: CPlaceInner::VarLane(
-                                local,
-                                var,
-                                lane_idx.bits().try_into().unwrap(),
-                            ),
-                            layout: elem_layout,
-                        };
-                    }
                     CPlaceInner::Addr(addr, None) => (elem_layout, addr),
-                    CPlaceInner::Addr(_, Some(_))
-                    | CPlaceInner::VarPair(_, _, _)
-                    | CPlaceInner::VarLane(_, _, _) => bug!("Can't index into {self:?}"),
+                    CPlaceInner::Var(_, _)
+                    | CPlaceInner::Addr(_, Some(_))
+                    | CPlaceInner::VarPair(_, _, _) => bug!("Can't index into {self:?}"),
                 }
-                // FIXME use VarLane in case of Var with simd type
             }
             ty::Slice(elem_ty) => (fx.layout_of(*elem_ty), self.to_ptr_maybe_unsized().0),
             _ => bug!("place_index({:?})", self.layout().ty),

Original file line number	Diff line number	Diff line change
`@@ -100,7 +100,6 @@ pub(super) fn add_local_place_comments<'tcx>(`
`100`	`100`	`assert_eq!(local, place_local);`
`101`	`101`	`("ssa", Cow::Owned(format!("var=({}, {})", var1.index(), var2.index())))`
`102`	`102`	`}`
`103`		`- CPlaceInner::VarLane(_local, _var, _lane) => unreachable!(),`
`104`	`103`	`CPlaceInner::Addr(ptr, meta) => {`
`105`	`104`	`let meta = if let Some(meta) = meta {`
`106`	`105`	`Cow::Owned(format!("meta={}", meta))`
Original file line number	Diff line number	Diff line change
`@@ -84,7 +84,7 @@ impl<'tcx> ArgAbiExt<'tcx> for ArgAbi<'tcx, Ty<'tcx>> {`
`84`	`84`	`attrs`
`85`	`85`	`)],`
`86`	`86`	`Abi::Vector { .. } => {`
`87`		`- let vector_ty = crate::intrinsics::clif_vector_type(tcx, self.layout).unwrap();`
	`87`	`+ let vector_ty = crate::intrinsics::clif_vector_type(tcx, self.layout);`
`88`	`88`	`smallvec![AbiParam::new(vector_ty)]`
`89`	`89`	`}`
`90`	`90`	`_ => unreachable!("{:?}", self.layout.abi),`
`@@ -135,7 +135,7 @@ impl<'tcx> ArgAbiExt<'tcx> for ArgAbi<'tcx, Ty<'tcx>> {`
`135`	`135`	`(None, vec![AbiParam::new(scalar_to_clif_type(tcx, scalar))])`
`136`	`136`	`}`
`137`	`137`	`Abi::Vector { .. } => {`
`138`		`- let vector_ty = crate::intrinsics::clif_vector_type(tcx, self.layout).unwrap();`
	`138`	`+ let vector_ty = crate::intrinsics::clif_vector_type(tcx, self.layout);`
`139`	`139`	`(None, vec![AbiParam::new(vector_ty)])`
`140`	`140`	`}`
`141`	`141`	`_ => unreachable!("{:?}", self.layout.abi),`
Original file line number	Diff line number	Diff line change
`@@ -253,7 +253,7 @@ pub(super) fn codegen_simd_intrinsic_call<'tcx>(`
`253`	`253`	`}`
`254`	`254`
`255`	`255`	`ret.write_cvalue(fx, base);`
`256`		`- let ret_lane = ret.place_field(fx, mir::Field::new(idx.try_into().unwrap()));`
	`256`	`+ let ret_lane = ret.place_lane(fx, idx.try_into().unwrap());`
`257`	`257`	`ret_lane.write_cvalue(fx, val);`
`258`	`258`	`}`
`259`	`259`