Allow constant references (to constant data), when they're valid SPIR-V.

This commit is contained in:
Eduard-Mihai Burtescu 2021-04-09 02:42:06 +03:00 committed by Eduard-Mihai Burtescu
parent 2bf4f76741
commit 7c568ab219
9 changed files with 282 additions and 120 deletions

View File

@ -838,8 +838,7 @@ impl<'a, 'tcx> BuilderMethods<'a, 'tcx> for Builder<'a, 'tcx> {
}
fn load(&mut self, ptr: Self::Value, _align: Align) -> Self::Value {
// See comment on `SpirvValueKind::ConstantPointer`
if let Some(value) = ptr.const_ptr_val(self) {
if let Some(value) = ptr.const_fold_load(self) {
return value;
}
let ty = match self.lookup_type(ptr.ty) {
@ -1665,9 +1664,7 @@ impl<'a, 'tcx> BuilderMethods<'a, 'tcx> for Builder<'a, 'tcx> {
};
let src_element_size = src_pointee.and_then(|p| self.lookup_type(p).sizeof(self));
if src_element_size.is_some() && src_element_size == const_size.map(Size::from_bytes) {
// See comment on `SpirvValueKind::ConstantPointer`
if let Some(const_value) = src.const_ptr_val(self) {
if let Some(const_value) = src.const_fold_load(self) {
self.store(const_value, dst, Align::from_bytes(0).unwrap());
} else {
self.emit()

View File

@ -2,7 +2,7 @@ use crate::builder;
use crate::codegen_cx::CodegenCx;
use crate::spirv_type::SpirvType;
use rspirv::dr::{Block, Builder, Module, Operand};
use rspirv::spirv::{AddressingModel, Capability, MemoryModel, Op, Word};
use rspirv::spirv::{AddressingModel, Capability, MemoryModel, Op, StorageClass, Word};
use rspirv::{binary::Assemble, binary::Disassemble};
use rustc_data_structures::fx::FxHashMap;
use rustc_middle::bug;
@ -23,27 +23,10 @@ pub enum SpirvValueKind {
// 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).
// FIXME(eddyb) document? not sure what to do with the `ConstantPointer` comment.
FnAddr {
function: Word,
},
/// There are a fair number of places where `rustc_codegen_ssa` creates a pointer to something
/// that cannot be pointed to in SPIR-V. For example, constant values are frequently emitted as
/// a pointer to constant memory, and then dereferenced where they're used. Functions are the
/// same way, when compiling a call, the function's pointer is loaded, then dereferenced, then
/// called. Directly translating these constructs is impossible, because SPIR-V doesn't allow
/// pointers to constants, or function pointers. So, instead, we create this ConstantPointer
/// "meta-value": directly using it is an error, however, if it is attempted to be
/// dereferenced, the "load" is instead a no-op that returns the underlying value directly.
ConstantPointer {
initializer: Word,
/// The global (module-scoped) `OpVariable` (with `initializer` set as
/// its initializer) to attach zombies to.
global_var: 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).
///
@ -70,23 +53,29 @@ pub struct SpirvValue {
}
impl SpirvValue {
pub fn const_ptr_val(self, cx: &CodegenCx<'_>) -> Option<Self> {
pub fn const_fold_load(self, cx: &CodegenCx<'_>) -> Option<Self> {
match self.kind {
SpirvValueKind::ConstantPointer {
initializer,
global_var: _,
} => {
SpirvValueKind::Def(id) | SpirvValueKind::IllegalConst(id) => {
let entry = cx.builder.id_to_const.borrow().get(&id)?.clone();
match entry.val {
SpirvConst::PtrTo { pointee } => {
let ty = match cx.lookup_type(self.ty) {
SpirvType::Pointer { pointee } => pointee,
ty => bug!("load called on variable that wasn't a pointer: {:?}", ty),
ty => bug!("load called on value that wasn't a pointer: {:?}", ty),
};
Some(initializer.with_type(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,
}
}
SpirvValueKind::FnAddr { .. }
| SpirvValueKind::Def(_)
| SpirvValueKind::IllegalConst(_)
| SpirvValueKind::LogicalPtrCast { .. } => None,
_ => None,
}
}
@ -108,26 +97,47 @@ impl SpirvValue {
SpirvValueKind::Def(id) => id,
SpirvValueKind::IllegalConst(id) => {
let illegal = cx.builder.id_to_const.borrow()[&id].legal.unwrap_err();
if cx.is_system_crate() {
cx.zombie_with_span(id, span, illegal.message());
} else {
cx.tcx
.sess
.struct_span_err(span, illegal.message())
.note(&format!("constant type: {}", cx.debug_type(self.ty)))
.emit();
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(),
};
// HACK(eddyb) we don't know whether this constant originated
// in a system crate, so it's better to always zombie.
cx.zombie_even_in_user_code(id, span, msg);
id
}
SpirvValueKind::FnAddr { .. } => {
if cx.is_system_crate() {
*cx.zombie_undefs_for_system_fn_addrs
cx.builder
.const_to_id
.borrow()
.get(&self.ty)
.get(&WithType {
ty: self.ty,
val: SpirvConst::ZombieUndefForFnAddr,
})
.expect("FnAddr didn't go through proper undef registration")
.val
} else {
cx.tcx
.sess
@ -138,21 +148,6 @@ impl SpirvValue {
}
}
SpirvValueKind::ConstantPointer {
initializer: _,
global_var,
} => {
// HACK(eddyb) we don't know whether this constant originated
// in a system crate, so it's better to always zombie.
cx.zombie_even_in_user_code(
global_var,
span,
"Cannot use this pointer directly, it must be dereferenced first",
);
global_var
}
SpirvValueKind::LogicalPtrCast {
original_ptr: _,
original_pointee_ty,
@ -201,9 +196,23 @@ pub enum SpirvConst {
/// f64 isn't hash, so store bits
F64(u64),
Bool(bool),
Composite(Rc<[Word]>),
Null,
Undef,
/// Like `Undef`, but cached separately to avoid `FnAddr` zombies accidentally
/// applying to non-zombie `Undef`s of the same types.
// FIXME(eddyb) include the function ID so that multiple `fn` pointers to
// different functions, but of the same type, don't overlap their zombies.
ZombieUndefForFnAddr,
Composite(Rc<[Word]>),
/// Pointer to constant data, i.e. `&pointee`, represented as an `OpVariable`
/// in the `Private` storage class, and with `pointee` as its initializer.
PtrTo {
pointee: Word,
},
}
#[derive(Debug, Clone, Ord, PartialOrd, Eq, PartialEq, Hash)]
@ -212,13 +221,38 @@ struct WithType<V> {
val: V,
}
/// Primary causes for a `SpirvConst` to be deemed illegal.
#[derive(Copy, Clone, Debug)]
enum IllegalConst {}
impl IllegalConst {
fn message(&self) -> &'static str {
match *self {}
enum LeafIllegalConst {
/// `SpirvConst::Composite` containing a `SpirvConst::PtrTo` as a field.
/// This is illegal because `OpConstantComposite` must have other constants
/// as its operands, and `OpVariable`s are never considered constant.
// FIXME(eddyb) figure out if this is an accidental omission in SPIR-V.
CompositeContainsPtrTo,
}
impl LeafIllegalConst {
fn message(&self) -> &'static str {
match *self {
Self::CompositeContainsPtrTo => {
"constant arrays/structs cannot contain pointers to other constants"
}
}
}
}
#[derive(Copy, Clone, Debug)]
enum IllegalConst {
/// This `SpirvConst` is (or contains) a "leaf" illegal constant. As there
/// is no indirection, some of these could still be materialized at runtime,
/// using e.g. `OpCompositeConstruct` instead of `OpConstantComposite`.
Shallow(LeafIllegalConst),
/// This `SpirvConst` is (or contains/points to) a `PtrTo` which points to
/// a "leaf" illegal constant. As the data would have to live for `'static`,
/// there is no way to materialize it as a pointer in SPIR-V. However, it
/// could still be legalized during codegen by e.g. folding loads from it.
Indirect(LeafIllegalConst),
}
#[derive(Copy, Clone, Debug)]
@ -384,6 +418,7 @@ impl BuilderSpirv {
let val_with_type = WithType { ty, val };
let mut builder = self.builder(BuilderCursor::default());
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 {
@ -404,9 +439,15 @@ impl BuilderSpirv {
builder.constant_false(ty)
}
}
SpirvConst::Composite(ref v) => builder.constant_composite(ty, v.iter().copied()),
SpirvConst::Null => builder.constant_null(ty),
SpirvConst::Undef => builder.undef(ty, None),
SpirvConst::Undef | SpirvConst::ZombieUndefForFnAddr => builder.undef(ty, None),
SpirvConst::Composite(ref v) => builder.constant_composite(ty, v.iter().copied()),
SpirvConst::PtrTo { pointee } => {
builder.variable(ty, None, StorageClass::Private, Some(pointee))
}
};
#[allow(clippy::match_same_arms)]
let legal = match val {
@ -425,11 +466,52 @@ impl BuilderSpirv {
Ok(())
}
SpirvConst::Composite(ref v) => v.iter().fold(Ok(()), |result, field| {
// FIXME(eddyb) combine `IllegalConst`s to account for some being
// "worse" than others.
result.and(self.id_to_const.borrow()[field].legal)
SpirvConst::ZombieUndefForFnAddr => {
// This can be considered legal as it's already marked as zombie.
// FIXME(eddyb) is it possible for the original zombie to lack a
// span, and should we go through `IllegalConst` in order to be
// able to attach a proper usesite span?
Ok(())
}
SpirvConst::Composite(ref v) => v.iter().fold(Ok(()), |composite_legal, field| {
let field_entry = &self.id_to_const.borrow()[field];
let field_legal_in_composite = field_entry.legal.and_then(|()| {
// `field` is itself some legal `SpirvConst`, but can we have
// it as part of an `OpConstantComposite`?
match field_entry.val {
SpirvConst::PtrTo { .. } => Err(IllegalConst::Shallow(
LeafIllegalConst::CompositeContainsPtrTo,
)),
_ => Ok(()),
}
});
match (composite_legal, field_legal_in_composite) {
(Ok(()), Ok(())) => Ok(()),
(Err(illegal), Ok(())) | (Ok(()), Err(illegal)) => Err(illegal),
// Combining two causes of an illegal `SpirvConst` has to
// take into account which is "worse", i.e. which imposes
// more restrictions on how the resulting value can be used.
// `Indirect` is worse than `Shallow` because it cannot be
// materialized at runtime in the same way `Shallow` can be.
(Err(illegal @ IllegalConst::Indirect(_)), Err(_))
| (Err(_), Err(illegal @ IllegalConst::Indirect(_)))
| (Err(illegal @ IllegalConst::Shallow(_)), Err(IllegalConst::Shallow(_))) => {
Err(illegal)
}
}
}),
SpirvConst::PtrTo { pointee } => match self.id_to_const.borrow()[&pointee].legal {
Ok(()) => Ok(()),
// `Shallow` becomes `Indirect` when placed behind a pointer.
Err(IllegalConst::Shallow(cause)) | Err(IllegalConst::Indirect(cause)) => {
Err(IllegalConst::Indirect(cause))
}
},
};
assert_matches!(
self.const_to_id.borrow_mut().insert(
@ -447,6 +529,7 @@ impl BuilderSpirv {
.insert(id, WithConstLegality { val, legal }),
None
);
// FIXME(eddyb) deduplicate this `if`-`else` and its other copies.
let kind = if legal.is_ok() {
SpirvValueKind::Def(id)
} else {

View File

@ -4,7 +4,7 @@ use crate::builder_spirv::{SpirvConst, SpirvValue, SpirvValueExt};
use crate::spirv_type::SpirvType;
use rspirv::spirv::Word;
use rustc_codegen_ssa::mir::place::PlaceRef;
use rustc_codegen_ssa::traits::{ConstMethods, MiscMethods, StaticMethods};
use rustc_codegen_ssa::traits::{BaseTypeMethods, ConstMethods, MiscMethods, StaticMethods};
use rustc_middle::bug;
use rustc_middle::mir::interpret::{AllocId, Allocation, GlobalAlloc, Pointer, ScalarMaybeUninit};
use rustc_middle::ty::layout::TyAndLayout;
@ -166,7 +166,12 @@ impl<'tcx> ConstMethods<'tcx> for CodegenCx<'tcx> {
.spirv_type(DUMMY_SP, self);
// FIXME(eddyb) include the actual byte data.
(
self.make_constant_pointer(DUMMY_SP, self.undef(str_ty)),
self.builder.def_constant(
self.type_ptr_to(str_ty),
SpirvConst::PtrTo {
pointee: self.undef(str_ty).def_cx(self),
},
),
self.const_usize(len as u64),
)
}

View File

@ -6,14 +6,14 @@ use crate::decorations::UnrollLoopsDecoration;
use crate::spirv_type::SpirvType;
use rspirv::spirv::{FunctionControl, LinkageType, StorageClass, Word};
use rustc_attr::InlineAttr;
use rustc_codegen_ssa::traits::{PreDefineMethods, StaticMethods};
use rustc_codegen_ssa::traits::{BaseTypeMethods, PreDefineMethods, StaticMethods};
use rustc_middle::bug;
use rustc_middle::middle::codegen_fn_attrs::{CodegenFnAttrFlags, CodegenFnAttrs};
use rustc_middle::mir::mono::{Linkage, MonoItem, Visibility};
use rustc_middle::ty::layout::FnAbiExt;
use rustc_middle::ty::{self, Instance, ParamEnv, TypeFoldable};
use rustc_span::def_id::DefId;
use rustc_span::{Span, DUMMY_SP};
use rustc_span::Span;
use rustc_target::abi::call::FnAbi;
use rustc_target::abi::{Align, LayoutOf};
@ -242,7 +242,12 @@ impl<'tcx> PreDefineMethods<'tcx> for CodegenCx<'tcx> {
impl<'tcx> StaticMethods for CodegenCx<'tcx> {
fn static_addr_of(&self, cv: Self::Value, _align: Align, _kind: Option<&str>) -> Self::Value {
self.make_constant_pointer(DUMMY_SP, cv)
self.builder.def_constant(
self.type_ptr_to(cv.ty),
SpirvConst::PtrTo {
pointee: cv.def_cx(self),
},
)
}
fn codegen_static(&self, def_id: DefId, _is_mutable: bool) {

View File

@ -4,14 +4,14 @@ mod entry;
mod type_;
use crate::builder::{ExtInst, InstructionTable};
use crate::builder_spirv::{BuilderCursor, BuilderSpirv, SpirvValue, SpirvValueKind};
use crate::builder_spirv::{BuilderCursor, BuilderSpirv, SpirvConst, SpirvValue, SpirvValueKind};
use crate::decorations::{
CustomDecoration, SerializedSpan, UnrollLoopsDecoration, ZombieDecoration,
};
use crate::spirv_type::{SpirvType, SpirvTypePrinter, TypeCache};
use crate::symbols::Symbols;
use rspirv::dr::{Module, Operand};
use rspirv::spirv::{AddressingModel, Decoration, LinkageType, MemoryModel, StorageClass, Word};
use rspirv::spirv::{AddressingModel, Decoration, LinkageType, MemoryModel, Word};
use rustc_codegen_ssa::mir::debuginfo::{FunctionDebugContext, VariableKind};
use rustc_codegen_ssa::traits::{
AsmMethods, BackendTypes, CoverageInfoMethods, DebugInfoMethods, MiscMethods,
@ -59,7 +59,6 @@ pub struct CodegenCx<'tcx> {
/// Cache of all the builtin symbols we need
pub sym: Rc<Symbols>,
pub instruction_table: InstructionTable,
pub zombie_undefs_for_system_fn_addrs: RefCell<FxHashMap<Word, Word>>,
pub libm_intrinsics: RefCell<FxHashMap<Word, super::builder::libm_intrinsics::LibmIntrinsic>>,
/// Simple `panic!("...")` and builtin panics (from MIR `Assert`s) call `#[lang = "panic"]`.
@ -120,7 +119,6 @@ impl<'tcx> CodegenCx<'tcx> {
kernel_mode,
sym,
instruction_table: InstructionTable::new(),
zombie_undefs_for_system_fn_addrs: Default::default(),
libm_intrinsics: Default::default(),
panic_fn_id: Default::default(),
panic_bounds_check_fn_id: Default::default(),
@ -230,36 +228,6 @@ impl<'tcx> CodegenCx<'tcx> {
once(Operand::LiteralString(name)).chain(once(Operand::LinkageType(linkage))),
)
}
/// See note on `SpirvValueKind::ConstantPointer`
pub fn make_constant_pointer(&self, span: Span, value: SpirvValue) -> SpirvValue {
let ty = SpirvType::Pointer { pointee: value.ty }.def(span, self);
let initializer = value.def_cx(self);
// Create these up front instead of on demand in SpirvValue::def because
// SpirvValue::def can't use cx.emit()
// FIXME(eddyb) figure out what the correct storage class is.
let global_var =
self.emit_global()
.variable(ty, None, StorageClass::Private, Some(initializer));
// In all likelihood, this zombie message will get overwritten in SpirvValue::def_with_span
// to the use site of this constant. However, if this constant happens to never get used, we
// still want to zobmie it, so zombie here.
self.zombie_even_in_user_code(
global_var,
span,
"Cannot use this pointer directly, it must be dereferenced first",
);
SpirvValue {
kind: SpirvValueKind::ConstantPointer {
initializer,
global_var,
},
ty,
}
}
}
pub struct CodegenArgs {
@ -377,13 +345,8 @@ impl<'tcx> MiscMethods<'tcx> for CodegenCx<'tcx> {
if self.is_system_crate() {
// Create these undefs up front instead of on demand in SpirvValue::def because
// SpirvValue::def can't use cx.emit()
self.zombie_undefs_for_system_fn_addrs
.borrow_mut()
.entry(ty)
.or_insert_with(|| {
// We want a unique ID for these undefs, so don't use the caching system.
self.emit_global().undef(ty, None)
});
self.builder
.def_constant(ty, SpirvConst::ZombieUndefForFnAddr);
}
SpirvValue {

View File

@ -0,0 +1,28 @@
// Test `&'static T` constants where the `T` values themselves contain references,
// nested in `OpConstantComposite` (structs/arrays) - currently these are disallowed.
// build-fail
use spirv_std as _;
use glam::{const_mat2, Mat2, Vec2};
#[inline(never)]
fn pair_deep_load(r: &'static (&'static u32, &'static f32)) -> (u32, f32) {
(*r.0, *r.1)
}
#[inline(never)]
fn array3_deep_load(r: &'static [&'static u32; 3]) -> [u32; 3] {
[*r[0], *r[1], *r[2]]
}
#[spirv(fragment)]
pub fn main_pair(pair_out: &mut (u32, f32)) {
*pair_out = pair_deep_load(&(&123, &3.14));
}
#[spirv(fragment)]
pub fn main_array3(array3_out: &mut [u32; 3]) {
*array3_out = array3_deep_load(&[&0, &1, &2]);
}

View File

@ -0,0 +1,27 @@
error: constant arrays/structs cannot contain pointers to other constants
--> $DIR/nested-ref-in-composite.rs:22:17
|
22 | *pair_out = pair_deep_load(&(&123, &3.14));
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: Stack:
nested_ref_in_composite::main_pair
Unnamed function ID %26
error: constant arrays/structs cannot contain pointers to other constants
--> $DIR/nested-ref-in-composite.rs:27:19
|
27 | *array3_out = array3_deep_load(&[&0, &1, &2]);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: Stack:
nested_ref_in_composite::main_array3
Unnamed function ID %33
error: invalid binary:0:0 - No OpEntryPoint instruction was found. This is only allowed if the Linkage capability is being used.
|
= note: spirv-val failed
= note: module `$TEST_BUILD_DIR/lang/consts/nested-ref-in-composite.stage-id.spv`
error: aborting due to 3 previous errors

View File

@ -0,0 +1,32 @@
// Test `&'static &'static T` constants where the `T` values don't themselves
// contain references, and where the `T` values aren't immediatelly loaded from.
// build-pass
use spirv_std as _;
use glam::{const_mat2, Mat2, Vec2};
#[inline(never)]
fn deep_load(r: &'static &'static u32) -> u32 {
**r
}
const ROT90: &Mat2 = &const_mat2![[0.0, 1.0], [-1.0, 0.0]];
#[inline(never)]
fn deep_transpose(r: &'static &'static Mat2) -> Mat2 {
r.transpose()
}
#[spirv(fragment)]
pub fn main(
scalar_out: &mut u32,
#[spirv(push_constant)] vec_in: &Vec2,
bool_out: &mut bool,
vec_out: &mut Vec2,
) {
*scalar_out = deep_load(&&123);
*bool_out = vec_in == &Vec2::ZERO;
*vec_out = deep_transpose(&ROT90) * *vec_in;
}

View File

@ -0,0 +1,22 @@
// Test `&'static T` constants where the `T` values don't themselves contain
// references, and where the `T` values aren't immediatelly loaded from.
// build-pass
use spirv_std as _;
use glam::{const_mat2, Mat2, Vec2};
#[inline(never)]
fn scalar_load(r: &'static u32) -> u32 {
*r
}
const ROT90: Mat2 = const_mat2![[0.0, 1.0], [-1.0, 0.0]];
#[spirv(fragment)]
pub fn main(scalar_out: &mut u32, vec_in: Vec2, bool_out: &mut bool, vec_out: &mut Vec2) {
*scalar_out = scalar_load(&123);
*bool_out = vec_in == Vec2::ZERO;
*vec_out = ROT90.transpose() * vec_in;
}