abi: make dig_scalar_pointee search for offsets in all layout fields.

crates/rustc_codegen_spirv/src/abi.rs

@@ -7,6 +7,7 @@ use crate::spirv_type::SpirvType;
 use rspirv::spirv::{Capability, StorageClass, Word};
 use rustc_data_structures::fx::FxHashMap;
 use rustc_errors::ErrorReported;
+use rustc_index::vec::Idx;
 use rustc_middle::bug;
 use rustc_middle::ty::layout::{FnAbiExt, TyAndLayout};
 use rustc_middle::ty::subst::SubstsRef;
@@ -18,7 +19,7 @@ use rustc_span::Span;
 use rustc_span::DUMMY_SP;
 use rustc_target::abi::call::{CastTarget, FnAbi, PassMode, Reg, RegKind};
 use rustc_target::abi::{
-    Abi, Align, FieldsShape, LayoutOf, Primitive, Scalar, Size, TagEncoding, VariantIdx, Variants,
+    Abi, Align, FieldsShape, LayoutOf, Primitive, Scalar, Size, VariantIdx, Variants,
 };
 use std::cell::RefCell;
 use std::collections::hash_map::Entry;
@@ -356,28 +357,28 @@ fn trans_type_impl<'tcx>(
             field_names: None,
         }
         .def_with_name(cx, span, TyLayoutNameKey::from(ty)),
-        Abi::Scalar(ref scalar) => trans_scalar(cx, span, ty, scalar, None, is_immediate),
-        Abi::ScalarPair(ref one, ref two) => {
-            // Note! Do not pass through is_immediate here - they're wrapped in a struct, hence, not immediate.
-            let one_spirv = trans_scalar(cx, span, ty, one, Some(0), false);
-            let two_spirv = trans_scalar(cx, span, ty, two, Some(1), false);
+        Abi::Scalar(ref scalar) => trans_scalar(cx, span, ty, scalar, Size::ZERO, is_immediate),
+        Abi::ScalarPair(ref a, ref b) => {
             // Note: We can't use auto_struct_layout here because the spirv types here might be undefined due to
             // recursive pointer types.
-            let one_offset = Size::ZERO;
-            let two_offset = one.value.size(cx).align_to(two.value.align(cx).abi);
+            let a_offset = Size::ZERO;
+            let b_offset = a.value.size(cx).align_to(b.value.align(cx).abi);
+            // Note! Do not pass through is_immediate here - they're wrapped in a struct, hence, not immediate.
+            let a = trans_scalar(cx, span, ty, a, a_offset, false);
+            let b = trans_scalar(cx, span, ty, b, b_offset, false);
             let size = if ty.is_unsized() { None } else { Some(ty.size) };
             SpirvType::Adt {
                 def_id: def_id_for_spirv_type_adt(ty),
                 size,
                 align: ty.align.abi,
-                field_types: vec![one_spirv, two_spirv],
-                field_offsets: vec![one_offset, two_offset],
+                field_types: vec![a, b],
+                field_offsets: vec![a_offset, b_offset],
                 field_names: None,
             }
             .def_with_name(cx, span, TyLayoutNameKey::from(ty))
         }
         Abi::Vector { ref element, count } => {
-            let elem_spirv = trans_scalar(cx, span, ty, element, None, false);
+            let elem_spirv = trans_scalar(cx, span, ty, element, Size::ZERO, false);
             SpirvType::Vector {
                 element: elem_spirv,
                 count: count as u32,
@@ -397,11 +398,16 @@ pub fn scalar_pair_element_backend_type<'tcx>(
     index: usize,
     is_immediate: bool,
 ) -> Word {
-    let scalar = match &ty.layout.abi {
-        Abi::ScalarPair(a, b) => [a, b][index],
+    let [a, b] = match &ty.layout.abi {
+        Abi::ScalarPair(a, b) => [a, b],
         other => bug!("scalar_pair_element_backend_type invalid abi: {:?}", other),
     };
-    trans_scalar(cx, span, ty, scalar, Some(index), is_immediate)
+    let offset = match index {
+        0 => Size::ZERO,
+        1 => a.value.size(cx).align_to(b.value.align(cx).abi),
+        _ => unreachable!(),
+    };
+    trans_scalar(cx, span, ty, [a, b][index], offset, is_immediate)
 }
 
 /// A "scalar" is a basic building block: bools, ints, floats, pointers. (i.e. not something complex like a struct)
@@ -416,7 +422,7 @@ fn trans_scalar<'tcx>(
     span: Span,
     ty: TyAndLayout<'tcx>,
     scalar: &Scalar,
-    index: Option<usize>,
+    offset: Size,
     is_immediate: bool,
 ) -> Word {
     if is_immediate && scalar.is_bool() {
@@ -433,7 +439,7 @@ fn trans_scalar<'tcx>(
         Primitive::F32 => SpirvType::Float(32).def(span, cx),
         Primitive::F64 => SpirvType::Float(64).def(span, cx),
         Primitive::Pointer => {
-            let pointee_ty = dig_scalar_pointee(cx, ty, index);
+            let pointee_ty = dig_scalar_pointee(cx, ty, offset);
             // Pointers can be recursive. So, record what we're currently translating, and if we're already translating
             // the same type, emit an OpTypeForwardPointer and use that ID.
             if let Some(predefined_result) = cx
@@ -464,113 +470,72 @@ fn trans_scalar<'tcx>(
 // If the above didn't make sense, please poke Ashley, it's probably easier to explain via conversation.
 fn dig_scalar_pointee<'tcx>(
     cx: &CodegenCx<'tcx>,
-    ty: TyAndLayout<'tcx>,
-    index: Option<usize>,
+    layout: TyAndLayout<'tcx>,
+    offset: Size,
 ) -> PointeeTy<'tcx> {
-    match *ty.ty.kind() {
-        TyKind::Ref(_, elem_ty, _) | TyKind::RawPtr(TypeAndMut { ty: elem_ty, .. }) => {
-            let elem = cx.layout_of(elem_ty);
-            match index {
-                None => PointeeTy::Ty(elem),
-                Some(index) => {
-                    if elem.is_unsized() {
-                        dig_scalar_pointee(cx, ty.field(cx, index), None)
-                    } else {
-                        // This can sometimes happen in weird cases when going through the Adt case below - an ABI
-                        // of ScalarPair could be deduced, but it's actually e.g. a sized pointer followed by some other
-                        // completely unrelated type, not a wide pointer. So, translate this as a single scalar, one
-                        // component of that ScalarPair.
-                        PointeeTy::Ty(elem)
-                    }
-                }
+    if let FieldsShape::Primitive = layout.fields {
+        assert_eq!(offset, Size::ZERO);
+        let pointee = match *layout.ty.kind() {
+            TyKind::Ref(_, pointee_ty, _) | TyKind::RawPtr(TypeAndMut { ty: pointee_ty, .. }) => {
+                PointeeTy::Ty(cx.layout_of(pointee_ty))
             }
-        }
-        TyKind::FnPtr(sig) if index.is_none() => PointeeTy::Fn(sig),
-        TyKind::Adt(def, _) if def.is_box() => {
-            let ptr_ty = cx.layout_of(cx.tcx.mk_mut_ptr(ty.ty.boxed_ty()));
-            dig_scalar_pointee(cx, ptr_ty, index)
-        }
-        TyKind::Tuple(_) | TyKind::Adt(..) | TyKind::Closure(..) => {
-            dig_scalar_pointee_adt(cx, ty, index)
-        }
-        ref kind => cx.tcx.sess.fatal(&format!(
-            "TODO: Unimplemented Primitive::Pointer TyKind index={:?} ({:#?}):\n{:#?}",
-            index, kind, ty
-        )),
+            TyKind::FnPtr(sig) => PointeeTy::Fn(sig),
+            _ => bug!("Pointer is not `&T`, `*T` or `fn` pointer: {:#?}", layout),
+        };
+        return pointee;
     }
-}
 
-fn dig_scalar_pointee_adt<'tcx>(
-    cx: &CodegenCx<'tcx>,
-    ty: TyAndLayout<'tcx>,
-    index: Option<usize>,
-) -> PointeeTy<'tcx> {
-    match &ty.variants {
-        // If it's a Variants::Multiple, then we want to emit the type of the dataful variant, not the type of the
-        // discriminant. This is because the discriminant can e.g. have type *mut(), whereas we want the full underlying
-        // type, only available in the dataful variant.
-        Variants::Multiple {
-            tag_encoding,
-            tag_field,
-            variants,
-            ..
-        } => {
-            match *tag_encoding {
-                TagEncoding::Direct => cx.tcx.sess.fatal(&format!(
-                    "dig_scalar_pointee_adt Variants::Multiple TagEncoding::Direct makes no sense: {:#?}",
-                    ty
-                )),
-                TagEncoding::Niche { dataful_variant, .. } => {
-                    // This *should* be something like Option<&T>: a very simple enum.
-                    // TODO: This might not be, if it's a scalar pair?
-                    assert_eq!(1, ty.fields.count());
-                    assert_eq!(1, variants[dataful_variant].fields.count());
-                    if let TyKind::Adt(adt, substs) = ty.ty.kind() {
-                        assert_eq!(1, adt.variants[dataful_variant].fields.len());
-                        assert_eq!(0, *tag_field);
-                        let field_ty = adt.variants[dataful_variant].fields[0].ty(cx.tcx, substs);
-                        dig_scalar_pointee(cx, cx.layout_of(field_ty), index)
-                    } else {
-                        bug!("Variants::Multiple not TyKind::Adt: {:#?}", ty)
-                    }
-                },
-            }
+    let all_fields = (match &layout.variants {
+        Variants::Multiple { variants, .. } => 0..variants.len(),
+        Variants::Single { index } => {
+            let i = index.as_usize();
+            i..i + 1
         }
-        Variants::Single { .. } => {
-            let fields = ty
-                .fields
-                .index_by_increasing_offset()
-                .map(|f| ty.field(cx, f))
-                .filter(|f| !f.is_zst())
-                .collect::<Vec<_>>();
-            match index {
-                Some(index) => match fields.len() {
-                    1 => dig_scalar_pointee(cx, fields[0], Some(index)),
-                    // This case right here is the cause of the comment handling TyKind::Ref.
-                    2 => dig_scalar_pointee(cx, fields[index], None),
-                    other => cx.tcx.sess.fatal(&format!(
-                        "Unable to dig scalar pair pointer type: fields length {}",
-                        other
-                    )),
-                },
-                None => match fields.len() {
-                    1 => dig_scalar_pointee(cx, fields[0], None),
-                    other => cx.tcx.sess.fatal(&format!(
-                        "Unable to dig scalar pointer type: fields length {}",
-                        other
-                    )),
-                },
+    })
+    .flat_map(|variant_idx| {
+        let variant = layout.for_variant(cx, VariantIdx::new(variant_idx));
+        (0..variant.fields.count()).map(move |field_idx| {
+            (
+                variant.field(cx, field_idx),
+                variant.fields.offset(field_idx),
+            )
+        })
+    });
+
+    let mut pointee = None;
+    for (field, field_offset) in all_fields {
+        if field.is_zst() {
+            continue;
+        }
+        if (field_offset..field_offset + field.size).contains(&offset) {
+            let new_pointee = dig_scalar_pointee(cx, field, offset - field_offset);
+            match pointee {
+                Some(old_pointee) if old_pointee != new_pointee => {
+                    cx.tcx.sess.fatal(&format!(
+                        "dig_scalar_pointee: unsupported Pointer with different \
+                         pointee types ({:?} vs {:?}) at offset {:?} in {:#?}",
+                        old_pointee, new_pointee, offset, layout
+                    ));
+                }
+                _ => pointee = Some(new_pointee),
             }
         }
     }
+    pointee.unwrap_or_else(|| {
+        bug!(
+            "field containing Pointer scalar at offset {:?} not found in {:#?}",
+            offset,
+            layout
+        )
+    })
 }
 
 fn trans_aggregate<'tcx>(cx: &CodegenCx<'tcx>, span: Span, ty: TyAndLayout<'tcx>) -> Word {
     match ty.fields {
-        FieldsShape::Primitive => cx.tcx.sess.fatal(&format!(
-            "FieldsShape::Primitive not supported yet in trans_type: {:?}",
+        FieldsShape::Primitive => bug!(
+            "trans_aggregate called for FieldsShape::Primitive layout {:#?}",
             ty
-        )),
+        ),
         FieldsShape::Union(_) => {
             assert_ne!(ty.size.bytes(), 0, "{:#?}", ty);
             assert!(!ty.is_unsized(), "{:#?}", ty);

crates/rustc_codegen_spirv/src/lib.rs

@@ -115,6 +115,7 @@ extern crate rustc_data_structures;
 extern crate rustc_driver;
 extern crate rustc_errors;
 extern crate rustc_hir;
+extern crate rustc_index;
 extern crate rustc_interface;
 extern crate rustc_middle;
 extern crate rustc_mir;

tests/ui/lang/core/ops/range-contains.rs

@@ -0,0 +1,19 @@
+// Test that using `(a..b).contains(&x)`, which is starting to get used
+// in `core` (see https://github.com/rust-lang/rust/pull/87723), cannot
+// cause a fatal error, but at most a zombie or SPIR-V validation error.
+
+// build-fail
+
+// HACK(eddyb) this allows CI (older?) `spirv-val` output to also work.
+// normalize-stderr-test "  %\d+ = OpVariable %\w+ Function\n\n" -> ""
+
+use spirv_std as _;
+
+fn has_two_decimal_digits(x: u32) -> bool {
+    (10..100).contains(&x)
+}
+
+#[spirv(fragment)]
+pub fn main(i: u32, o: &mut bool) {
+    *o = has_two_decimal_digits(i);
+}

tests/ui/lang/core/ops/range-contains.stderr

@@ -0,0 +1,7 @@
+error: error:0:0 - In Logical addressing, variables may not allocate a pointer type
+  |
+  = note: spirv-val failed
+  = note: module `$TEST_BUILD_DIR/lang/core/ops/range-contains.stage-id.spv.dir/module`
+
+error: aborting due to previous error
+