diff --git a/crates/rustc_codegen_spirv/src/abi.rs b/crates/rustc_codegen_spirv/src/abi.rs index 0c5cf69f4d..e86822fe82 100644 --- a/crates/rustc_codegen_spirv/src/abi.rs +++ b/crates/rustc_codegen_spirv/src/abi.rs @@ -616,15 +616,6 @@ fn trans_aggregate<'tcx>(cx: &CodegenCx<'tcx>, span: Span, ty: TyAndLayout<'tcx> } } -#[cfg_attr( - not(rustc_codegen_spirv_disable_pqp_cg_ssa), - expect( - unused, - reason = "actually used from \ - `>::const_struct`, \ - but `rustc_codegen_ssa` being `pqp_cg_ssa` makes that trait unexported" - ) -)] // returns (field_offsets, size, align) pub fn auto_struct_layout( cx: &CodegenCx<'_>, diff --git a/crates/rustc_codegen_spirv/src/builder/builder_methods.rs b/crates/rustc_codegen_spirv/src/builder/builder_methods.rs index 7b6de35187..5962adbabe 100644 --- a/crates/rustc_codegen_spirv/src/builder/builder_methods.rs +++ b/crates/rustc_codegen_spirv/src/builder/builder_methods.rs @@ -26,6 +26,7 @@ use rustc_codegen_ssa::traits::{ }; use rustc_middle::bug; use rustc_middle::middle::codegen_fn_attrs::CodegenFnAttrs; +use rustc_middle::ty::layout::LayoutOf; use rustc_middle::ty::{self, AtomicOrdering, Ty}; use rustc_span::Span; use rustc_target::callconv::FnAbi; @@ -1721,30 +1722,15 @@ impl<'a, 'tcx> BuilderMethods<'a, 'tcx> for Builder<'a, 'tcx> { fn checked_binop( &mut self, oop: OverflowOp, - ty: Ty<'_>, + ty: Ty<'tcx>, lhs: Self::Value, rhs: Self::Value, ) -> (Self::Value, Self::Value) { - // adopted partially from https://github.com/ziglang/zig/blob/master/src/codegen/spirv.zig - let is_add = match oop { - OverflowOp::Add => true, - OverflowOp::Sub => false, - OverflowOp::Mul => { - // NOTE(eddyb) this needs to be `undef`, not `false`/`true`, because - // we don't want the user's boolean constants to keep the zombie alive. - let bool = SpirvType::Bool.def(self.span(), self); - let overflowed = self.undef(bool); - - let result = (self.mul(lhs, rhs), overflowed); - self.zombie(result.1.def(self), "checked mul is not supported yet"); - return result; - } - }; let signed = match ty.kind() { ty::Int(_) => true, ty::Uint(_) => false, - other => self.fatal(format!( - "Unexpected {} type: {other:#?}", + _ => self.fatal(format!( + "unexpected {} type: {ty}", match oop { OverflowOp::Add => "checked add", OverflowOp::Sub => "checked sub", @@ -1753,13 +1739,17 @@ impl<'a, 'tcx> BuilderMethods<'a, 'tcx> for Builder<'a, 'tcx> { )), }; - let result = if is_add { - self.add(lhs, rhs) - } else { - self.sub(lhs, rhs) - }; + // HACK(eddyb) SPIR-V `OpIAddCarry`/`OpISubBorrow` are specifically for + // unsigned overflow, so signed overflow still needs this custom logic. + if signed && let OverflowOp::Add | OverflowOp::Sub = oop { + let result = match oop { + OverflowOp::Add => self.add(lhs, rhs), + OverflowOp::Sub => self.sub(lhs, rhs), + OverflowOp::Mul => unreachable!(), + }; + + // adopted partially from https://github.com/ziglang/zig/blob/master/src/codegen/spirv.zig - let overflowed = if signed { // when adding, overflow could happen if // - rhs is positive and result < lhs; or // - rhs is negative and result > lhs @@ -1771,30 +1761,80 @@ impl<'a, 'tcx> BuilderMethods<'a, 'tcx> for Builder<'a, 'tcx> { // this is equivalent to (rhs < 0) == (result < lhs) let rhs_lt_zero = self.icmp(IntPredicate::IntSLT, rhs, self.constant_int(rhs.ty, 0)); let result_gt_lhs = self.icmp( - if is_add { - IntPredicate::IntSGT - } else { - IntPredicate::IntSLT + match oop { + OverflowOp::Add => IntPredicate::IntSGT, + OverflowOp::Sub => IntPredicate::IntSLT, + OverflowOp::Mul => unreachable!(), }, result, lhs, ); - self.icmp(IntPredicate::IntEQ, rhs_lt_zero, result_gt_lhs) - } else { - // for unsigned addition, overflow occurred if the result is less than any of the operands. - // for subtraction, overflow occurred if the result is greater. - self.icmp( - if is_add { - IntPredicate::IntULT + + let overflowed = self.icmp(IntPredicate::IntEQ, rhs_lt_zero, result_gt_lhs); + + return (result, overflowed); + } + + let result_type = self.layout_of(ty).spirv_type(self.span(), self); + let pair_result_type = { + let field_types = [result_type, result_type]; + let (field_offsets, size, align) = crate::abi::auto_struct_layout(self, &field_types); + SpirvType::Adt { + def_id: None, + size, + align, + field_types: &field_types, + field_offsets: &field_offsets, + field_names: None, + } + .def(self.span(), self) + }; + + let lhs = lhs.def(self); + let rhs = rhs.def(self); + let pair_result = match oop { + OverflowOp::Add => self + .emit() + .i_add_carry(pair_result_type, None, lhs, rhs) + .unwrap(), + OverflowOp::Sub => self + .emit() + .i_sub_borrow(pair_result_type, None, lhs, rhs) + .unwrap(), + OverflowOp::Mul => { + if signed { + self.emit() + .s_mul_extended(pair_result_type, None, lhs, rhs) + .unwrap() } else { - IntPredicate::IntUGT - }, - result, - lhs, - ) + self.emit() + .u_mul_extended(pair_result_type, None, lhs, rhs) + .unwrap() + } + } + } + .with_type(pair_result_type); + let result_lo = self.extract_value(pair_result, 0); + let result_hi = self.extract_value(pair_result, 1); + + // HACK(eddyb) SPIR-V lacks any `(T, T) -> (T, bool)` instructions, + // so instead `result_hi` is compared with the value expected in the + // non-overflow case (`0`, or `-1` for negative signed multiply result). + let expected_nonoverflowing_hi = match (oop, signed) { + (OverflowOp::Add | OverflowOp::Sub, _) | (OverflowOp::Mul, false) => { + self.const_uint(result_type, 0) + } + (OverflowOp::Mul, true) => { + // HACK(eddyb) `(x: iN) >> (N - 1)` will spread the sign bit + // across all `N` bits of `iN`, and should be equivalent to + // `if x < 0 { -1 } else { 0 }`, without needing compare+select). + let result_width = u32::try_from(self.int_width(result_type)).unwrap(); + self.ashr(result_lo, self.const_u32(result_width - 1)) + } }; + let overflowed = self.icmp(IntPredicate::IntNE, result_hi, expected_nonoverflowing_hi); - (result, overflowed) + (result_lo, overflowed) } // rustc has the concept of an immediate vs. memory type - bools are compiled to LLVM bools as @@ -1854,7 +1894,12 @@ impl<'a, 'tcx> BuilderMethods<'a, 'tcx> for Builder<'a, 'tcx> { order: AtomicOrdering, _size: Size, ) -> Self::Value { - let (ptr, access_ty) = self.adjust_pointer_for_typed_access(ptr, ty); + // HACK(eddyb) SPIR-V lacks pointer atomics, have to use integers instead. + let atomic_ty = match self.lookup_type(ty) { + SpirvType::Pointer { .. } => self.type_usize(), + _ => ty, + }; + let (ptr, access_ty) = self.adjust_pointer_for_typed_access(ptr, atomic_ty); // TODO: Default to device scope let memory = self.constant_u32(self.span(), Scope::Device as u32); @@ -1991,7 +2036,12 @@ impl<'a, 'tcx> BuilderMethods<'a, 'tcx> for Builder<'a, 'tcx> { order: AtomicOrdering, _size: Size, ) { - let (ptr, access_ty) = self.adjust_pointer_for_typed_access(ptr, val.ty); + // HACK(eddyb) SPIR-V lacks pointer atomics, have to use integers instead. + let atomic_ty = match self.lookup_type(val.ty) { + SpirvType::Pointer { .. } => self.type_usize(), + _ => val.ty, + }; + let (ptr, access_ty) = self.adjust_pointer_for_typed_access(ptr, atomic_ty); let val = self.bitcast(val, access_ty); // TODO: Default to device scope @@ -2390,13 +2440,6 @@ impl<'a, 'tcx> BuilderMethods<'a, 'tcx> for Builder<'a, 'tcx> { #[instrument(level = "trace", skip(self), fields(ptr, ptr_ty = ?self.debug_type(ptr.ty), dest_ty = ?self.debug_type(dest_ty)))] fn pointercast(&mut self, ptr: Self::Value, dest_ty: Self::Type) -> Self::Value { - // HACK(eddyb) reuse the special-casing in `const_bitcast`, which relies - // on adding a pointer type to an untyped pointer (to some const data). - if let SpirvValueKind::IllegalConst(_) = ptr.kind { - trace!("illegal const"); - return self.const_bitcast(ptr, dest_ty); - } - if ptr.ty == dest_ty { trace!("ptr.ty == dest_ty"); return ptr; @@ -2455,13 +2498,43 @@ impl<'a, 'tcx> BuilderMethods<'a, 'tcx> for Builder<'a, 'tcx> { self.debug_type(ptr_pointee), self.debug_type(dest_pointee), ); + + // HACK(eddyb) reuse the special-casing in `const_bitcast`, which relies + // on adding a pointer type to an untyped pointer (to some const data). + if self.builder.lookup_const(ptr).is_some() { + // FIXME(eddyb) remove the condition on `zombie_waiting_for_span`, + // and constant-fold all pointer bitcasts, regardless of "legality", + // once `strip_ptrcasts` can undo `const_bitcast`, as well. + if ptr.zombie_waiting_for_span { + trace!("illegal const"); + return self.const_bitcast(ptr, dest_ty); + } + } + // Defer the cast so that it has a chance to be avoided. - let original_ptr = ptr.def(self); + let ptr_id = ptr.def(self); + let bitcast_result_id = self.emit().bitcast(dest_ty, None, ptr_id).unwrap(); + + self.zombie( + bitcast_result_id, + &format!( + "cannot cast between pointer types\ + \nfrom `{}`\ + \n to `{}`", + self.debug_type(ptr.ty), + self.debug_type(dest_ty) + ), + ); + SpirvValue { - kind: SpirvValueKind::LogicalPtrCast { - original_ptr, - original_ptr_ty: ptr.ty, - bitcast_result_id: self.emit().bitcast(dest_ty, None, original_ptr).unwrap(), + zombie_waiting_for_span: false, + kind: SpirvValueKind::Def { + id: bitcast_result_id, + original_ptr_before_casts: Some(SpirvValue { + zombie_waiting_for_span: ptr.zombie_waiting_for_span, + kind: ptr_id, + ty: ptr.ty, + }), }, ty: dest_ty, } @@ -3091,7 +3164,12 @@ impl<'a, 'tcx> BuilderMethods<'a, 'tcx> for Builder<'a, 'tcx> { assert_ty_eq!(self, cmp.ty, src.ty); let ty = src.ty; - let (dst, access_ty) = self.adjust_pointer_for_typed_access(dst, ty); + // HACK(eddyb) SPIR-V lacks pointer atomics, have to use integers instead. + let atomic_ty = match self.lookup_type(ty) { + SpirvType::Pointer { .. } => self.type_usize(), + _ => ty, + }; + let (dst, access_ty) = self.adjust_pointer_for_typed_access(dst, atomic_ty); let cmp = self.bitcast(cmp, access_ty); let src = self.bitcast(src, access_ty); @@ -3117,7 +3195,7 @@ impl<'a, 'tcx> BuilderMethods<'a, 'tcx> for Builder<'a, 'tcx> { .with_type(access_ty); let val = self.bitcast(result, ty); - let success = self.icmp(IntPredicate::IntEQ, val, cmp); + let success = self.icmp(IntPredicate::IntEQ, result, cmp); (val, success) } @@ -3131,7 +3209,12 @@ impl<'a, 'tcx> BuilderMethods<'a, 'tcx> for Builder<'a, 'tcx> { ) -> Self::Value { let ty = src.ty; - let (dst, access_ty) = self.adjust_pointer_for_typed_access(dst, ty); + // HACK(eddyb) SPIR-V lacks pointer atomics, have to use integers instead. + let atomic_ty = match self.lookup_type(ty) { + SpirvType::Pointer { .. } => self.type_usize(), + _ => ty, + }; + let (dst, access_ty) = self.adjust_pointer_for_typed_access(dst, atomic_ty); let src = self.bitcast(src, access_ty); self.validate_atomic(access_ty, dst.def(self)); @@ -3279,7 +3362,7 @@ impl<'a, 'tcx> BuilderMethods<'a, 'tcx> for Builder<'a, 'tcx> { return_type, arguments, } => ( - if let SpirvValueKind::FnAddr { function } = callee.kind { + if let SpirvValueKind::FnAddr { function, .. } = callee.kind { assert_ty_eq!(self, callee_ty, pointee); function } @@ -3351,10 +3434,7 @@ impl<'a, 'tcx> BuilderMethods<'a, 'tcx> for Builder<'a, 'tcx> { if buffer_store_intrinsic { self.codegen_buffer_store_intrinsic(fn_abi, args); let void_ty = SpirvType::Void.def(rustc_span::DUMMY_SP, self); - return SpirvValue { - kind: SpirvValueKind::IllegalTypeUsed(void_ty), - ty: void_ty, - }; + return self.undef(void_ty); } if let Some((source_ty, target_ty)) = from_trait_impl { diff --git a/crates/rustc_codegen_spirv/src/builder/byte_addressable_buffer.rs b/crates/rustc_codegen_spirv/src/builder/byte_addressable_buffer.rs index 00ffaf661b..c0269b4bbf 100644 --- a/crates/rustc_codegen_spirv/src/builder/byte_addressable_buffer.rs +++ b/crates/rustc_codegen_spirv/src/builder/byte_addressable_buffer.rs @@ -2,7 +2,7 @@ use crate::maybe_pqp_cg_ssa as rustc_codegen_ssa; use super::Builder; -use crate::builder_spirv::{SpirvValue, SpirvValueExt, SpirvValueKind}; +use crate::builder_spirv::{SpirvValue, SpirvValueExt}; use crate::spirv_type::SpirvType; use rspirv::spirv::{Decoration, Word}; use rustc_abi::{Align, Size}; @@ -188,12 +188,8 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { ) -> SpirvValue { let pass_mode = &fn_abi.unwrap().ret.mode; match pass_mode { - PassMode::Ignore => { - return SpirvValue { - kind: SpirvValueKind::IllegalTypeUsed(result_type), - ty: result_type, - }; - } + PassMode::Ignore => return self.undef(result_type), + // PassMode::Pair is identical to PassMode::Direct - it's returned as a struct PassMode::Direct(_) | PassMode::Pair(_, _) => (), PassMode::Cast { .. } => { diff --git a/crates/rustc_codegen_spirv/src/builder/format_args_decompiler.rs b/crates/rustc_codegen_spirv/src/builder/format_args_decompiler.rs index f04cf79f12..7dc8baf334 100644 --- a/crates/rustc_codegen_spirv/src/builder/format_args_decompiler.rs +++ b/crates/rustc_codegen_spirv/src/builder/format_args_decompiler.rs @@ -93,11 +93,11 @@ impl<'tcx> DecodedFormatArgs<'tcx> { // HACK(eddyb) some entry-points only take a `&str`, not `fmt::Arguments`. if let [ SpirvValue { - kind: SpirvValueKind::Def(a_id), + kind: SpirvValueKind::Def { id: a_id, .. }, .. }, SpirvValue { - kind: SpirvValueKind::Def(b_id), + kind: SpirvValueKind::Def { id: b_id, .. }, .. }, ref other_args @ .., @@ -116,14 +116,20 @@ impl<'tcx> DecodedFormatArgs<'tcx> { // HACK(eddyb) `panic_nounwind_fmt` takes an extra argument. [ SpirvValue { - kind: SpirvValueKind::Def(format_args_id), + kind: + SpirvValueKind::Def { + id: format_args_id, .. + }, .. }, _, // `&'static panic::Location<'static>` ] | [ SpirvValue { - kind: SpirvValueKind::Def(format_args_id), + kind: + SpirvValueKind::Def { + id: format_args_id, .. + }, .. }, _, // `force_no_backtrace: bool` diff --git a/crates/rustc_codegen_spirv/src/builder/intrinsics.rs b/crates/rustc_codegen_spirv/src/builder/intrinsics.rs index 0fcaec8513..b4bedd8d56 100644 --- a/crates/rustc_codegen_spirv/src/builder/intrinsics.rs +++ b/crates/rustc_codegen_spirv/src/builder/intrinsics.rs @@ -115,33 +115,39 @@ impl<'a, 'tcx> IntrinsicCallBuilderMethods<'tcx> for Builder<'a, 'tcx> { sym::saturating_add => { assert_eq!(arg_tys[0], arg_tys[1]); - let result = match arg_tys[0].kind() { - TyKind::Int(_) | TyKind::Uint(_) => { - self.add(args[0].immediate(), args[1].immediate()) - } - TyKind::Float(_) => self.fadd(args[0].immediate(), args[1].immediate()), + match arg_tys[0].kind() { + TyKind::Int(_) | TyKind::Uint(_) => self + .emit() + .i_add_sat_intel( + ret_ty, + None, + args[0].immediate().def(self), + args[1].immediate().def(self), + ) + .unwrap() + .with_type(ret_ty), other => self.fatal(format!( - "Unimplemented saturating_add intrinsic type: {other:#?}" + "unimplemented saturating_add intrinsic type: {other:#?}" )), - }; - // TODO: Implement this - self.zombie(result.def(self), "saturating_add is not implemented yet"); - result + } } sym::saturating_sub => { assert_eq!(arg_tys[0], arg_tys[1]); - let result = match &arg_tys[0].kind() { - TyKind::Int(_) | TyKind::Uint(_) => { - self.sub(args[0].immediate(), args[1].immediate()) - } - TyKind::Float(_) => self.fsub(args[0].immediate(), args[1].immediate()), + match &arg_tys[0].kind() { + TyKind::Int(_) | TyKind::Uint(_) => self + .emit() + .i_sub_sat_intel( + ret_ty, + None, + args[0].immediate().def(self), + args[1].immediate().def(self), + ) + .unwrap() + .with_type(ret_ty), other => self.fatal(format!( - "Unimplemented saturating_sub intrinsic type: {other:#?}" + "unimplemented saturating_sub intrinsic type: {other:#?}" )), - }; - // TODO: Implement this - self.zombie(result.def(self), "saturating_sub is not implemented yet"); - result + } } sym::sqrtf32 | sym::sqrtf64 | sym::sqrtf128 => { diff --git a/crates/rustc_codegen_spirv/src/builder_spirv.rs b/crates/rustc_codegen_spirv/src/builder_spirv.rs index e05b433057..f430dfdf7f 100644 --- a/crates/rustc_codegen_spirv/src/builder_spirv.rs +++ b/crates/rustc_codegen_spirv/src/builder_spirv.rs @@ -16,7 +16,6 @@ use rustc_abi::Size; use rustc_arena::DroplessArena; use rustc_codegen_ssa::traits::ConstCodegenMethods as _; use rustc_data_structures::fx::{FxHashMap, FxHashSet}; -use rustc_middle::bug; use rustc_middle::mir::interpret::ConstAllocation; use rustc_middle::ty::TyCtxt; use rustc_span::source_map::SourceMap; @@ -31,91 +30,86 @@ use std::str; use std::sync::Arc; use std::{fs::File, io::Write, path::Path}; +// HACK(eddyb) silence warnings that are inaccurate wrt future changes. +#[non_exhaustive] #[derive(Copy, Clone, Debug, Ord, PartialOrd, Eq, PartialEq, Hash)] pub enum SpirvValueKind { - Def(Word), - - /// The ID of a global instruction matching a `SpirvConst`, but which cannot - /// pass validation. Used to error (or attach zombie spans), at the usesites - /// of such constants, instead of where they're generated (and cached). - IllegalConst(Word), - - /// This can only happen in one specific case - which is as a result of - /// `codegen_buffer_store_intrinsic`, that function is supposed to return - /// `OpTypeVoid`, however because it gets inline by the compiler it can't. - /// Instead we return this, and trigger an error if we ever end up using the - /// result of this function call (which we can't). - IllegalTypeUsed(Word), + Def { + id: Word, + + /// If `id` is a pointer cast, this will be `Some`, and contain all the + /// information necessary to regenerate the original `SpirvValue` before + /// *any* pointer casts were applied, effectively deferring the casts + /// (as long as all downstream uses apply `.strip_ptrcasts()` first), + /// and bypassing errors they might cause (due to SPIR-V limitations). + // + // FIXME(eddyb) wouldn't it be easier to use this for *any* bitcasts? + // (with some caveats around dedicated int<->ptr casts vs bitcasts) + original_ptr_before_casts: Option>, + }, // FIXME(eddyb) this shouldn't be needed, but `rustc_codegen_ssa` still relies // on converting `Function`s to `Value`s even for direct calls, the `Builder` // should just have direct and indirect `call` variants (or a `Callee` enum). FnAddr { function: Word, - }, - - /// Deferred pointer cast, for the `Logical` addressing model (which doesn't - /// really support raw pointers in the way Rust expects to be able to use). - /// - /// The cast's target pointer type is the `ty` of the `SpirvValue` that has - /// `LogicalPtrCast` as its `kind`, as it would be redundant to have it here. - LogicalPtrCast { - /// Pointer value being cast. - original_ptr: Word, - - /// Pointer type of `original_ptr`. - original_ptr_ty: Word, - /// Result ID for the `OpBitcast` instruction representing the cast, - /// to attach zombies to. - // - // HACK(eddyb) having an `OpBitcast` only works by being DCE'd away, - // or by being replaced with a noop in `qptr::lower`. - bitcast_result_id: Word, + // FIXME(eddyb) replace this ad-hoc zombie with a proper `SpirvConst`. + zombie_id: Word, }, } #[derive(Copy, Clone, Debug, Ord, PartialOrd, Eq, PartialEq, Hash)] -pub struct SpirvValue { - pub kind: SpirvValueKind, +pub struct SpirvValue { + // HACK(eddyb) used to cheaply check whether this is a SPIR-V value ID + // with a "zombie" (deferred error) attached to it, that may need a `Span` + // still (e.g. such as constants, which can't easily take a `Span`). + // FIXME(eddyb) a whole `bool` field is sadly inefficient, but anything + // which may make `SpirvValue` smaller requires far too much impl effort. + pub zombie_waiting_for_span: bool, + + pub kind: K, pub ty: Word, } +impl SpirvValue { + fn map_kind(self, f: impl FnOnce(K) -> K2) -> SpirvValue { + let SpirvValue { + zombie_waiting_for_span, + kind, + ty, + } = self; + SpirvValue { + zombie_waiting_for_span, + kind: f(kind), + ty, + } + } +} + impl SpirvValue { pub fn strip_ptrcasts(self) -> Self { match self.kind { - SpirvValueKind::LogicalPtrCast { - original_ptr, - original_ptr_ty, - bitcast_result_id: _, - } => original_ptr.with_type(original_ptr_ty), + SpirvValueKind::Def { + id: _, + original_ptr_before_casts: Some(original_ptr), + } => original_ptr.map_kind(|id| SpirvValueKind::Def { + id, + original_ptr_before_casts: None, + }), _ => self, } } pub fn const_fold_load(self, cx: &CodegenCx<'_>) -> Option { - match self.kind { - SpirvValueKind::Def(id) | SpirvValueKind::IllegalConst(id) => { - let &entry = cx.builder.id_to_const.borrow().get(&id)?; - match entry.val { - SpirvConst::PtrTo { pointee } => { - let ty = match cx.lookup_type(self.ty) { - SpirvType::Pointer { pointee } => pointee, - ty => bug!("load called on value that wasn't a pointer: {:?}", ty), - }; - // FIXME(eddyb) deduplicate this `if`-`else` and its other copies. - let kind = if entry.legal.is_ok() { - SpirvValueKind::Def(pointee) - } else { - SpirvValueKind::IllegalConst(pointee) - }; - Some(SpirvValue { kind, ty }) - } - _ => None, - } + match cx.builder.lookup_const(self)? { + SpirvConst::PtrTo { pointee } => { + // HACK(eddyb) this obtains a `SpirvValue` from the ID it contains, + // so there's some conceptual inefficiency there, but it does + // prevent any of the other details from being lost accidentally. + Some(cx.builder.id_to_const_and_val.borrow().get(&pointee)?.val.1) } - _ => None, } } @@ -134,80 +128,13 @@ impl SpirvValue { } pub fn def_with_span(self, cx: &CodegenCx<'_>, span: Span) -> Word { - match self.kind { - SpirvValueKind::Def(id) => id, - - SpirvValueKind::IllegalConst(id) => { - let entry = &cx.builder.id_to_const.borrow()[&id]; - let msg = match entry.legal.unwrap_err() { - IllegalConst::Shallow(cause) => { - if let ( - LeafIllegalConst::CompositeContainsPtrTo, - SpirvConst::Composite(_fields), - ) = (cause, &entry.val) - { - // FIXME(eddyb) materialize this at runtime, using - // `OpCompositeConstruct` (transitively, i.e. after - // putting every field through `SpirvValue::def`), - // if we have a `Builder` to do that in. - // FIXME(eddyb) this isn't possible right now, as - // the builder would be dynamically "locked" anyway - // (i.e. attempting to do `bx.emit()` would panic). - } - - cause.message() - } - - IllegalConst::Indirect(cause) => cause.message(), - }; - - cx.zombie_with_span(id, span, msg); - - id - } - - SpirvValueKind::IllegalTypeUsed(id) => { - cx.tcx - .dcx() - .struct_span_err(span, "Can't use type as a value") - .with_note(format!("Type: *{}", cx.debug_type(id))) - .emit(); - - id - } - - SpirvValueKind::FnAddr { .. } => { - cx.builder - .const_to_id - .borrow() - .get(&WithType { - ty: self.ty, - val: SpirvConst::ZombieUndefForFnAddr, - }) - .expect("FnAddr didn't go through proper undef registration") - .val - } - - SpirvValueKind::LogicalPtrCast { - original_ptr: _, - original_ptr_ty, - bitcast_result_id, - } => { - cx.zombie_with_span( - bitcast_result_id, - span, - &format!( - "cannot cast between pointer types\ - \nfrom `{}`\ - \n to `{}`", - cx.debug_type(original_ptr_ty), - cx.debug_type(self.ty) - ), - ); - - bitcast_result_id - } + let id = match self.kind { + SpirvValueKind::Def { id, .. } | SpirvValueKind::FnAddr { zombie_id: id, .. } => id, + }; + if self.zombie_waiting_for_span { + cx.add_span_to_zombie_if_missing(id, span); } + id } } @@ -218,7 +145,11 @@ pub trait SpirvValueExt { impl SpirvValueExt for Word { fn with_type(self, ty: Word) -> SpirvValue { SpirvValue { - kind: SpirvValueKind::Def(self), + zombie_waiting_for_span: false, + kind: SpirvValueKind::Def { + id: self, + original_ptr_before_casts: None, + }, ty, } } @@ -436,11 +367,12 @@ pub struct BuilderSpirv<'tcx> { builder: RefCell, // Bidirectional maps between `SpirvConst` and the ID of the defined global - // (e.g. `OpConstant...`) instruction. - // NOTE(eddyb) both maps have `WithConstLegality` around their keys, which - // allows getting that legality information without additional lookups. - const_to_id: RefCell>, WithConstLegality>>, - id_to_const: RefCell>>>, + // (e.g. `OpConstant...`) instruction, with additional information in values + // (i.e. each map is keyed by only some part of the other map's value type), + // as needed to streamline operations (e.g. avoiding rederiving `SpirvValue`). + const_to_val: RefCell>, SpirvValue>>, + id_to_const_and_val: + RefCell, SpirvValue)>>>, debug_file_cache: RefCell>>, @@ -511,8 +443,8 @@ impl<'tcx> BuilderSpirv<'tcx> { source_map: tcx.sess.source_map(), dropless_arena: &tcx.arena.dropless, builder: RefCell::new(builder), - const_to_id: Default::default(), - id_to_const: Default::default(), + const_to_val: Default::default(), + id_to_const_and_val: Default::default(), debug_file_cache: Default::default(), enabled_capabilities, } @@ -616,14 +548,8 @@ impl<'tcx> BuilderSpirv<'tcx> { }; let val_with_type = WithType { ty, val }; - if let Some(entry) = self.const_to_id.borrow().get(&val_with_type) { - // FIXME(eddyb) deduplicate this `if`-`else` and its other copies. - let kind = if entry.legal.is_ok() { - SpirvValueKind::Def(entry.val) - } else { - SpirvValueKind::IllegalConst(entry.val) - }; - return SpirvValue { kind, ty }; + if let Some(&v) = self.const_to_val.borrow().get(&val_with_type) { + return v; } let val = val_with_type.val; @@ -755,11 +681,11 @@ impl<'tcx> BuilderSpirv<'tcx> { SpirvConst::Composite(v) => v .iter() .map(|field| { - let field_entry = &self.id_to_const.borrow()[field]; + let field_entry = &self.id_to_const_and_val.borrow()[field]; field_entry.legal.and( // `field` is itself some legal `SpirvConst`, but can we have // it as part of an `OpConstantComposite`? - match field_entry.val { + match field_entry.val.0 { SpirvConst::PtrTo { .. } => Err(IllegalConst::Shallow( LeafIllegalConst::CompositeContainsPtrTo, )), @@ -787,50 +713,71 @@ impl<'tcx> BuilderSpirv<'tcx> { }) .unwrap_or(Ok(())), - SpirvConst::PtrTo { pointee } => match self.id_to_const.borrow()[&pointee].legal { - Ok(()) => Ok(()), + SpirvConst::PtrTo { pointee } => { + match self.id_to_const_and_val.borrow()[&pointee].legal { + Ok(()) => Ok(()), - // `Shallow` becomes `Indirect` when placed behind a pointer. - Err(IllegalConst::Shallow(cause) | IllegalConst::Indirect(cause)) => { - Err(IllegalConst::Indirect(cause)) + // `Shallow` becomes `Indirect` when placed behind a pointer. + Err(IllegalConst::Shallow(cause) | IllegalConst::Indirect(cause)) => { + Err(IllegalConst::Indirect(cause)) + } } - }, + } SpirvConst::ConstDataFromAlloc(_) => Err(IllegalConst::Shallow( LeafIllegalConst::UntypedConstDataFromAlloc, )), }; + + // FIXME(eddyb) avoid dragging "const (il)legality" around, as well + // (sadly that does require that `SpirvConst` -> SPIR-V be injective, + // e.g. `OpUndef` can never be used for unrepresentable constants). + if let Err(illegal) = legal { + let msg = match illegal { + IllegalConst::Shallow(cause) | IllegalConst::Indirect(cause) => cause.message(), + }; + cx.zombie_no_span(id, msg); + } + let val = val.tcx_arena_alloc_slices(cx); + + // FIXME(eddyb) the `val`/`v` name clash is a bit unfortunate. + let v = SpirvValue { + zombie_waiting_for_span: legal.is_err(), + kind: SpirvValueKind::Def { + id, + original_ptr_before_casts: None, + }, + ty, + }; + assert_matches!( - self.const_to_id + self.const_to_val .borrow_mut() - .insert(WithType { ty, val }, WithConstLegality { val: id, legal }), + .insert(WithType { ty, val }, v), None ); assert_matches!( - self.id_to_const - .borrow_mut() - .insert(id, WithConstLegality { val, legal }), + self.id_to_const_and_val.borrow_mut().insert( + id, + WithConstLegality { + val: (val, v), + legal + } + ), None ); - // FIXME(eddyb) deduplicate this `if`-`else` and its other copies. - let kind = if legal.is_ok() { - SpirvValueKind::Def(id) - } else { - SpirvValueKind::IllegalConst(id) - }; - SpirvValue { kind, ty } + + v } pub fn lookup_const_by_id(&self, id: Word) -> Option> { - Some(self.id_to_const.borrow().get(&id)?.val) + Some(self.id_to_const_and_val.borrow().get(&id)?.val.0) } pub fn lookup_const(&self, def: SpirvValue) -> Option> { match def.kind { - SpirvValueKind::Def(id) | SpirvValueKind::IllegalConst(id) => { - self.lookup_const_by_id(id) - } + SpirvValueKind::Def { id, .. } => self.lookup_const_by_id(id), _ => None, } } diff --git a/crates/rustc_codegen_spirv/src/codegen_cx/constant.rs b/crates/rustc_codegen_spirv/src/codegen_cx/constant.rs index acd9b42172..1bc8164dc6 100644 --- a/crates/rustc_codegen_spirv/src/codegen_cx/constant.rs +++ b/crates/rustc_codegen_spirv/src/codegen_cx/constant.rs @@ -3,7 +3,7 @@ use crate::maybe_pqp_cg_ssa as rustc_codegen_ssa; use super::CodegenCx; use crate::abi::ConvSpirvType; -use crate::builder_spirv::{SpirvConst, SpirvValue, SpirvValueExt, SpirvValueKind}; +use crate::builder_spirv::{SpirvConst, SpirvValue, SpirvValueExt}; use crate::spirv_type::SpirvType; use itertools::Itertools as _; use rspirv::spirv::Word; @@ -336,8 +336,7 @@ impl<'tcx> CodegenCx<'tcx> { pub fn const_bitcast(&self, val: SpirvValue, ty: Word) -> SpirvValue { // HACK(eddyb) special-case `const_data_from_alloc` + `static_addr_of` // as the old `from_const_alloc` (now `OperandRef::from_const_alloc`). - if let SpirvValueKind::IllegalConst(_) = val.kind - && let Some(SpirvConst::PtrTo { pointee }) = self.builder.lookup_const(val) + if let Some(SpirvConst::PtrTo { pointee }) = self.builder.lookup_const(val) && let Some(SpirvConst::ConstDataFromAlloc(alloc)) = self.builder.lookup_const_by_id(pointee) && let SpirvType::Pointer { pointee } = self.lookup_type(ty) diff --git a/crates/rustc_codegen_spirv/src/codegen_cx/mod.rs b/crates/rustc_codegen_spirv/src/codegen_cx/mod.rs index c8acc4bc1d..f36d8a624c 100644 --- a/crates/rustc_codegen_spirv/src/codegen_cx/mod.rs +++ b/crates/rustc_codegen_spirv/src/codegen_cx/mod.rs @@ -245,9 +245,9 @@ impl<'tcx> CodegenCx<'tcx> { /// is stripped from the binary. /// /// Errors will only be emitted (by `linker::zombies`) for reachable zombies. - pub fn zombie_with_span(&self, word: Word, span: Span, reason: &str) { + pub fn zombie_with_span(&self, id: Word, span: Span, reason: &str) { self.zombie_decorations.borrow_mut().insert( - word, + id, ( ZombieDecoration { // FIXME(eddyb) this could take advantage of `Cow` and use @@ -258,8 +258,16 @@ impl<'tcx> CodegenCx<'tcx> { ), ); } - pub fn zombie_no_span(&self, word: Word, reason: &str) { - self.zombie_with_span(word, DUMMY_SP, reason); + pub fn zombie_no_span(&self, id: Word, reason: &str) { + self.zombie_with_span(id, DUMMY_SP, reason); + } + + pub fn add_span_to_zombie_if_missing(&self, id: Word, span: Span) { + if span != DUMMY_SP + && let Some((_, src_loc @ None)) = self.zombie_decorations.borrow_mut().get_mut(&id) + { + *src_loc = SrcLocDecoration::from_rustc_span(span, &self.builder); + } } pub fn finalize_module(self) -> Module { @@ -331,16 +339,19 @@ pub struct CodegenArgs { impl CodegenArgs { pub fn from_session(sess: &Session) -> Self { - match CodegenArgs::parse(&sess.opts.cg.llvm_args) { + match Self::parse(sess, &sess.opts.cg.llvm_args) { Ok(ok) => ok, + + // FIXME(eddyb) this should mention `RUSTGPU_CODEGEN_ARGS`, just + // like how `RUSTGPU_CODEGEN_ARGS=--help` is already special-cased. Err(err) => sess .dcx() .fatal(format!("Unable to parse llvm-args: {err}")), } } - // FIXME(eddyb) `structopt` would come a long way to making this nicer. - pub fn parse(args: &[String]) -> Result { + // FIXME(eddyb) switch all of this over to `clap`. + pub fn parse(sess: &Session, args: &[String]) -> Result { use rustc_session::getopts; // FIXME(eddyb) figure out what casing ("Foo bar" vs "foo bar") to use @@ -459,6 +470,12 @@ impl CodegenArgs { "dump the SPIR-T module across passes, to a (pair of) file(s) in DIR", "DIR", ); + opts.optopt( + "", + "dump-spirt", + "dump the final SPIR-T module, to a (pair of) file(s) in DIR", + "DIR", + ); opts.optflag( "", "spirt-strip-custom-debuginfo-from-dumps", @@ -627,7 +644,19 @@ impl CodegenArgs { dump_pre_inline: matches_opt_dump_dir_path("dump-pre-inline"), dump_post_inline: matches_opt_dump_dir_path("dump-post-inline"), dump_post_split: matches_opt_dump_dir_path("dump-post-split"), - dump_spirt_passes: matches_opt_dump_dir_path("dump-spirt-passes"), + dump_spirt: match ["dump-spirt-passes", "dump-spirt"].map(matches_opt_dump_dir_path) { + [Some(dump_spirt_passes), dump_spirt] => { + if dump_spirt.is_some() { + sess.dcx() + .warn("`--dump-spirt` ignored in favor of `--dump-spirt-passes`"); + } + Some((dump_spirt_passes, crate::linker::DumpSpirtMode::AllPasses)) + } + [None, Some(dump_spirt)] => { + Some((dump_spirt, crate::linker::DumpSpirtMode::OnlyFinal)) + } + [None, None] => None, + }, spirt_strip_custom_debuginfo_from_dumps: matches .opt_present("spirt-strip-custom-debuginfo-from-dumps"), spirt_keep_debug_sources_in_dumps: matches @@ -846,11 +875,15 @@ impl<'tcx> MiscCodegenMethods<'tcx> for CodegenCx<'tcx> { // Create these `OpUndef`s up front, instead of on-demand in `SpirvValue::def`, // because `SpirvValue::def` can't use `cx.emit()`. - self.def_constant(ty, SpirvConst::ZombieUndefForFnAddr); + let zombie_id = self + .def_constant(ty, SpirvConst::ZombieUndefForFnAddr) + .def_with_span(self, span); SpirvValue { + zombie_waiting_for_span: false, kind: SpirvValueKind::FnAddr { function: function.id, + zombie_id, }, ty, } diff --git a/crates/rustc_codegen_spirv/src/linker/duplicates.rs b/crates/rustc_codegen_spirv/src/linker/duplicates.rs index 76f85a7713..70554caddd 100644 --- a/crates/rustc_codegen_spirv/src/linker/duplicates.rs +++ b/crates/rustc_codegen_spirv/src/linker/duplicates.rs @@ -283,7 +283,20 @@ pub fn remove_duplicate_debuginfo(module: &mut Module) { }) .map(|inst| inst.result_id.unwrap()); + let deduper = DebuginfoDeduplicator { + custom_ext_inst_set_import, + }; for func in &mut module.functions { + deduper.remove_duplicate_debuginfo_in_function(func); + } +} + +pub struct DebuginfoDeduplicator { + pub custom_ext_inst_set_import: Option, +} + +impl DebuginfoDeduplicator { + pub fn remove_duplicate_debuginfo_in_function(&self, func: &mut rspirv::dr::Function) { for block in &mut func.blocks { // Ignore the terminator, it's effectively "outside" debuginfo. let (_, insts) = block.instructions.split_last_mut().unwrap(); @@ -339,7 +352,8 @@ pub fn remove_duplicate_debuginfo(module: &mut Module) { let inst = &insts[inst_idx]; let custom_op = match inst.class.opcode { Op::ExtInst - if Some(inst.operands[0].unwrap_id_ref()) == custom_ext_inst_set_import => + if Some(inst.operands[0].unwrap_id_ref()) + == self.custom_ext_inst_set_import => { Some(CustomOp::decode_from_ext_inst(inst)) } diff --git a/crates/rustc_codegen_spirv/src/linker/inline.rs b/crates/rustc_codegen_spirv/src/linker/inline.rs index 0ab19bd40f..072b3d8b92 100644 --- a/crates/rustc_codegen_spirv/src/linker/inline.rs +++ b/crates/rustc_codegen_spirv/src/linker/inline.rs @@ -133,12 +133,57 @@ pub fn inline(sess: &Session, module: &mut Module) -> super::Result<()> { .map(Ok) .collect(); - // Inline functions in post-order (aka inside-out aka bottom-out) - that is, + let mut mem2reg_pointer_to_pointee = FxHashMap::default(); + let mut mem2reg_constants = FxHashMap::default(); + { + let mut u32 = None; + for inst in &module.types_global_values { + match inst.class.opcode { + Op::TypePointer => { + mem2reg_pointer_to_pointee + .insert(inst.result_id.unwrap(), inst.operands[1].unwrap_id_ref()); + } + Op::TypeInt + if inst.operands[0].unwrap_literal_bit32() == 32 + && inst.operands[1].unwrap_literal_bit32() == 0 => + { + assert!(u32.is_none()); + u32 = Some(inst.result_id.unwrap()); + } + Op::Constant if u32.is_some() && inst.result_type == u32 => { + let value = inst.operands[0].unwrap_literal_bit32(); + mem2reg_constants.insert(inst.result_id.unwrap(), value); + } + _ => {} + } + } + } + + // Inline functions in post-order (aka inside-out aka bottom-up) - that is, // callees are processed before their callers, to avoid duplicating work. for func_idx in call_graph.post_order() { let mut function = mem::replace(&mut functions[func_idx], Err(FuncIsBeingInlined)).unwrap(); inliner.inline_fn(&mut function, &functions); fuse_trivial_branches(&mut function); + + super::duplicates::DebuginfoDeduplicator { + custom_ext_inst_set_import, + } + .remove_duplicate_debuginfo_in_function(&mut function); + + { + super::simple_passes::block_ordering_pass(&mut function); + // Note: mem2reg requires functions to be in RPO order (i.e. block_ordering_pass) + super::mem2reg::mem2reg( + inliner.header, + &mut module.types_global_values, + &mem2reg_pointer_to_pointee, + &mem2reg_constants, + &mut function, + ); + super::destructure_composites::destructure_composites(&mut function); + } + functions[func_idx] = Ok(function); } @@ -411,7 +456,7 @@ fn should_inline( } // If the call isn't passing a legal pointer argument (a "memory object", - // i.e. an `OpVariable` or one of the caller's `OpFunctionParameters), + // i.e. an `OpVariable` or one of the caller's `OpFunctionParameter`s), // then inlining is required to have a chance at producing legal SPIR-V. // // FIXME(eddyb) rewriting away the pointer could be another alternative. @@ -428,6 +473,7 @@ fn should_inline( .caller .parameters .iter() + .filter(|_| false) .chain( call_site.caller.blocks[0] .instructions @@ -528,22 +574,22 @@ impl Inliner<'_, '_> { ) { let mut block_idx = 0; while block_idx < function.blocks.len() { - // If we successfully inlined a block, then repeat processing on the same block, in - // case the newly inlined block has more inlined calls. - // TODO: This is quadratic - if !self.inline_block(function, block_idx, functions) { - // TODO(eddyb) skip past the inlined callee without rescanning it. - block_idx += 1; + match self.inline_block(function, block_idx, functions) { + Some(post_call_block_idx) => block_idx = post_call_block_idx, + None => block_idx += 1, } } } + // HACK(eddyb) returns `Some(post_call_block_idx)` in case of success, where + // `post_call_block_idx` is the index of the first block of the `caller`, + // that follows all the inlined callee's blocks. fn inline_block( &mut self, caller: &mut Function, block_idx: usize, functions: &[Result], - ) -> bool { + ) -> Option { // Find the first inlined OpFunctionCall let call = caller.blocks[block_idx] .instructions @@ -583,10 +629,7 @@ impl Inliner<'_, '_> { } } }); - let (call_index, call_inst, callee) = match call { - None => return false, - Some(call) => call, - }; + let (call_index, call_inst, callee) = call?; // Propagate "may abort" from callee to caller (i.e. as aborts get inlined). if self @@ -738,8 +781,8 @@ impl Inliner<'_, '_> { } // Insert the post-call block, after all the inlined callee blocks. + let post_call_block_idx = pre_call_block_idx + num_non_entry_inlined_callee_blocks + 1; { - let post_call_block_idx = pre_call_block_idx + num_non_entry_inlined_callee_blocks + 1; let post_call_block = Block { label: Some(Instruction::new(Op::Label, None, Some(return_jump), vec![])), instructions: post_call_block_insts, @@ -826,7 +869,7 @@ impl Inliner<'_, '_> { } // `vars_and_debuginfo_range.end` indicates where `OpVariable`s - // end and other instructions start (module debuginfo), but to + // end and other instructions start (modulo debuginfo), but to // split the block in two, both sides of the "cut" need "repair": // - the variables are missing "inlined call frames" pops, that // may happen later in the block, and have to be synthesized @@ -882,7 +925,7 @@ impl Inliner<'_, '_> { ); } - true + Some(post_call_block_idx) } fn add_clone_id_rules(&mut self, rewrite_rules: &mut FxHashMap, blocks: &[Block]) { diff --git a/crates/rustc_codegen_spirv/src/linker/mem2reg.rs b/crates/rustc_codegen_spirv/src/linker/mem2reg.rs index df2434d63d..c6fd8c084e 100644 --- a/crates/rustc_codegen_spirv/src/linker/mem2reg.rs +++ b/crates/rustc_codegen_spirv/src/linker/mem2reg.rs @@ -193,6 +193,8 @@ fn insert_phis_all( for (var_map, _) in &var_maps_and_types { split_copy_memory(header, blocks, var_map); } + + let mut rewrite_rules = FxHashMap::default(); for &(ref var_map, base_var_type) in &var_maps_and_types { let blocks_with_phi = insert_phis(blocks, dominance_frontier, var_map); let mut renamer = Renamer { @@ -205,16 +207,15 @@ fn insert_phis_all( phi_defs: FxHashSet::default(), visited: FxHashSet::default(), stack: Vec::new(), - rewrite_rules: FxHashMap::default(), + rewrite_rules: &mut rewrite_rules, }; renamer.rename(0, None); - // FIXME(eddyb) shouldn't this full rescan of the function be done once? - apply_rewrite_rules( - &renamer.rewrite_rules, - blocks.values_mut().map(|block| &mut **block), - ); - remove_nops(blocks); } + apply_rewrite_rules( + &rewrite_rules, + blocks.values_mut().map(|block| &mut **block), + ); + remove_nops(blocks); remove_old_variables(blocks, &var_maps_and_types); true } @@ -443,7 +444,7 @@ struct Renamer<'a, 'b> { phi_defs: FxHashSet, visited: FxHashSet, stack: Vec, - rewrite_rules: FxHashMap, + rewrite_rules: &'a mut FxHashMap, } impl Renamer<'_, '_> { diff --git a/crates/rustc_codegen_spirv/src/linker/mod.rs b/crates/rustc_codegen_spirv/src/linker/mod.rs index d289c8e5fc..487f2eae78 100644 --- a/crates/rustc_codegen_spirv/src/linker/mod.rs +++ b/crates/rustc_codegen_spirv/src/linker/mod.rs @@ -61,13 +61,19 @@ pub struct Options { pub dump_pre_inline: Option, pub dump_post_inline: Option, pub dump_post_split: Option, - pub dump_spirt_passes: Option, + pub dump_spirt: Option<(PathBuf, DumpSpirtMode)>, pub spirt_strip_custom_debuginfo_from_dumps: bool, pub spirt_keep_debug_sources_in_dumps: bool, pub spirt_keep_unstructured_cfg_in_dumps: bool, pub specializer_dump_instances: Option, } +#[derive(Copy, Clone, PartialEq, Eq)] +pub enum DumpSpirtMode { + AllPasses, + OnlyFinal, +} + pub enum LinkResult { SingleModule(Box), MultipleModules { @@ -103,7 +109,7 @@ fn apply_rewrite_rules<'a>( ) }); for id in all_ids_mut { - if let Some(&rewrite) = rewrite_rules.get(id) { + while let Some(&rewrite) = rewrite_rules.get(id) { *id = rewrite; } } @@ -520,7 +526,7 @@ pub fn link( drop(timer); let pass_name = dump_guard.in_progress_pass_name.take().unwrap(); if let Some(module) = module - && opts.dump_spirt_passes.is_some() + && matches!(opts.dump_spirt, Some((_, DumpSpirtMode::AllPasses))) { dump_guard .per_pass_module_for_dumping @@ -801,9 +807,9 @@ impl Drop for SpirtDumpGuard<'_> { let mut dump_spirt_file_path = self.linker_options - .dump_spirt_passes + .dump_spirt .as_ref() - .map(|dump_dir| { + .map(|(dump_dir, _)| { dump_dir .join(self.disambiguated_crate_name_for_dumps) .with_extension("spirt") @@ -815,10 +821,6 @@ impl Drop for SpirtDumpGuard<'_> { // but that requires keeping around e.g. the initial SPIR-V for longer, // and probably invoking the "SPIR-T pipeline" here, as looping is hard). if self.any_spirt_bugs && dump_spirt_file_path.is_none() { - if self.per_pass_module_for_dumping.is_empty() { - self.per_pass_module_for_dumping - .push(("".into(), self.module.clone())); - } dump_spirt_file_path = Some(self.outputs.temp_path_for_diagnostic("spirt")); } @@ -826,6 +828,11 @@ impl Drop for SpirtDumpGuard<'_> { return; }; + if self.per_pass_module_for_dumping.is_empty() { + self.per_pass_module_for_dumping + .push(("".into(), self.module.clone())); + } + for (_, module) in &mut self.per_pass_module_for_dumping { // FIXME(eddyb) consider catching panics in this? self.linker_options.spirt_cleanup_for_dumping(module); @@ -835,7 +842,16 @@ impl Drop for SpirtDumpGuard<'_> { let versions = self .per_pass_module_for_dumping .iter() - .map(|(pass_name, module)| (format!("after {pass_name}"), module)); + .map(|(pass_name, module)| { + ( + if pass_name.is_empty() { + "".into() + } else { + format!("after {pass_name}") + }, + module, + ) + }); let mut panicked_printing_after_passes = None; for truncate_version_count in (1..=versions.len()).rev() { @@ -896,7 +912,11 @@ impl Drop for SpirtDumpGuard<'_> { "pretty-printed SPIR-T was saved to {}.html", dump_spirt_file_path.display() )); - if self.linker_options.dump_spirt_passes.is_none() { + let is_dumping_spirt_passes = matches!( + self.linker_options.dump_spirt, + Some((_, DumpSpirtMode::AllPasses)) + ); + if !is_dumping_spirt_passes { note.help("re-run with `RUSTGPU_CODEGEN_ARGS=\"--dump-spirt-passes=$PWD\"` for more details"); } note.note("pretty-printed SPIR-T is preferred when reporting Rust-GPU issues"); diff --git a/docs/src/codegen-args.md b/docs/src/codegen-args.md index 71fb69d6d2..687baf0cf0 100644 --- a/docs/src/codegen-args.md +++ b/docs/src/codegen-args.md @@ -175,6 +175,14 @@ _Note: passes that are not already enabled by default are considered experimenta Dump the `SPIR-🇹` module across passes (i.e. all of the versions before/after each pass), as a combined report, to a pair of files (`.spirt` and `.spirt.html`) in `DIR`. (the `.spirt.html` version of the report is the recommended form for viewing, as it uses tabling for versions, syntax-highlighting-like styling, and use->def linking) +Mutually exclusive with `--dump-spirt` (this takes precedence over that). + +### `--dump-spirt DIR` + +Dump the `SPIR-🇹` module, similar to `--dump-spirt-passes`, but only the final version. + +Mutually exclusive with `--dump-spirt-passes` (which takes precedence over this). + ### `--spirt-strip-custom-debuginfo-from-dumps` When dumping (pretty-printed) `SPIR-🇹` (e.g. with `--dump-spirt-passes`), strip diff --git a/tests/compiletests/ui/dis/ptr_copy.normal.stderr b/tests/compiletests/ui/dis/ptr_copy.normal.stderr index dd442fbb40..0be4afe69e 100644 --- a/tests/compiletests/ui/dis/ptr_copy.normal.stderr +++ b/tests/compiletests/ui/dis/ptr_copy.normal.stderr @@ -28,6 +28,12 @@ LL | pub fn main(i: f32, o: &mut f32) { error: cannot cast between pointer types from `*f32` to `*struct () { }` + --> $CORE_SRC/ptr/mod.rs:625:34 + | +LL | src: *const () = src as *const (), + | ^^^^^^^^^^^^^^^^ + | +note: used from within `core::ptr::copy::` --> $CORE_SRC/ptr/mod.rs:621:9 | LL | / ub_checks::assert_unsafe_precondition!( @@ -37,6 +43,29 @@ LL | | "ptr::copy requires that both pointer arguments are aligned a LL | | && ub_checks::maybe_is_aligned_and_not_null(dst, align, zero_size) LL | | ); | |_________^ +note: called by `ptr_copy::copy_via_raw_ptr` + --> $DIR/ptr_copy.rs:28:18 + | +LL | unsafe { core::ptr::copy(src, dst, 1) } + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +note: called by `ptr_copy::main` + --> $DIR/ptr_copy.rs:33:5 + | +LL | copy_via_raw_ptr(&i, o); + | ^^^^^^^^^^^^^^^^^^^^^^^ +note: called by `main` + --> $DIR/ptr_copy.rs:32:8 + | +LL | pub fn main(i: f32, o: &mut f32) { + | ^^^^ + +error: cannot cast between pointer types + from `*f32` + to `*struct () { }` + --> $CORE_SRC/ptr/mod.rs:626:32 + | +LL | dst: *mut () = dst as *mut (), + | ^^^^^^^^^^^^^^ | note: used from within `core::ptr::copy::` --> $CORE_SRC/ptr/mod.rs:621:9 @@ -64,5 +93,5 @@ note: called by `main` LL | pub fn main(i: f32, o: &mut f32) { | ^^^^ -error: aborting due to 2 previous errors +error: aborting due to 3 previous errors