Skip to content

Commit

Permalink
[naga spv-in] Let BlockContext hold a &mut Module. (#6682)
Browse files Browse the repository at this point in the history 
Refactor `front::spv::BlockContext` so that, rather than holding
various shared and mutable references to various fields of the
`crate::Module` being constructed, it holds just a single `&mut
Module`, and accesses its fields through that.

This should allow the SPIR-V front to use utility functions like
`Module::generate_predeclared_type` as intended.

Fixes #6679.
  • Loading branch information
@jimblandy
jimblandy authored Dec 9, 2024
1 parent 41b42b2 commit ba1f042
Show file tree
Hide file tree
Showing 3 changed files with 103 additions and 105 deletions.
19 changes: 9 additions & 10 deletions naga/src/front/spv/function.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,9 @@ impl<I: Iterator<Item = u32>> super::Frontend<I> {
}
}

// Note the index this function's handle will be assigned, for tracing.
let function_index = module.functions.len();

// Read body
self.function_call_graph.add_node(fun_id);
let mut parameters_sampling =
Expand All @@ -122,14 +125,10 @@ impl<I: Iterator<Item = u32>> super::Frontend<I> {
body_for_label: Default::default(),
mergers: Default::default(),
bodies: Default::default(),
module,
function_id: fun_id,
expressions: &mut fun.expressions,
local_arena: &mut fun.local_variables,
const_arena: &mut module.constants,
overrides: &mut module.overrides,
global_expressions: &mut module.global_expressions,
type_arena: &module.types,
global_arena: &module.global_variables,
arguments: &fun.arguments,
parameter_sampling: &mut parameters_sampling,
};
Expand Down Expand Up @@ -167,7 +166,7 @@ impl<I: Iterator<Item = u32>> super::Frontend<I> {
if let Some(ref prefix) = self.options.block_ctx_dump_prefix {
let dump_suffix = match self.lookup_entry_point.get(&fun_id) {
Some(ep) => format!("block_ctx.{:?}-{}.txt", ep.stage, ep.name),
None => format!("block_ctx.Fun-{}.txt", module.functions.len()),
None => format!("block_ctx.Fun-{}.txt", function_index),
};
let dest = prefix.join(dump_suffix);
let dump = format!("{block_ctx:#?}");
Expand Down Expand Up @@ -580,10 +579,10 @@ impl<I: Iterator<Item = u32>> super::Frontend<I> {
impl BlockContext<'_> {
pub(super) fn gctx(&self) -> crate::proc::GlobalCtx {
crate::proc::GlobalCtx {
types: self.type_arena,
constants: self.const_arena,
overrides: self.overrides,
global_expressions: self.global_expressions,
types: &self.module.types,
constants: &self.module.constants,
overrides: &self.module.overrides,
global_expressions: &self.module.global_expressions,
}
}

Expand Down
128 changes: 66 additions & 62 deletions naga/src/front/spv/image.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,9 @@ impl super::BlockContext<'_> {
handle: Handle<crate::Expression>,
) -> Result<Handle<crate::Type>, Error> {
match self.expressions[handle] {
crate::Expression::GlobalVariable(handle) => Ok(self.global_arena[handle].ty),
crate::Expression::GlobalVariable(handle) => {
Ok(self.module.global_variables[handle].ty)
}
crate::Expression::FunctionArgument(i) => Ok(self.arguments[i as usize].ty),
crate::Expression::Access { base, .. } => Ok(self.get_image_expr_ty(base)?),
ref other => Err(Error::InvalidImageExpression(other.clone())),
Expand Down Expand Up @@ -64,7 +66,7 @@ fn extract_image_coordinates(
coordinate_ty: Handle<crate::Type>,
ctx: &mut super::BlockContext,
) -> (Handle<crate::Expression>, Option<Handle<crate::Expression>>) {
let (given_size, kind) = match ctx.type_arena[coordinate_ty].inner {
let (given_size, kind) = match ctx.module.types[coordinate_ty].inner {
crate::TypeInner::Scalar(Scalar { kind, .. }) => (None, kind),
crate::TypeInner::Vector {
size,
Expand All @@ -75,7 +77,8 @@ fn extract_image_coordinates(

let required_size = image_dim.required_coordinate_size();
let required_ty = required_size.map(|size| {
ctx.type_arena
ctx.module
.types
.get(&crate::Type {
name: None,
inner: crate::TypeInner::Vector {
Expand Down Expand Up @@ -287,7 +290,7 @@ impl<I: Iterator<Item = u32>> super::Frontend<I> {
let coord_handle =
self.get_expr_handle(coordinate_id, coord_lexp, ctx, emitter, block, body_idx);
let coord_type_handle = self.lookup_type.lookup(coord_lexp.type_id)?.handle;
let (coordinate, array_index) = match ctx.type_arena[image_ty].inner {
let (coordinate, array_index) = match ctx.module.types[image_ty].inner {
crate::TypeInner::Image {
dim,
arrayed,
Expand Down Expand Up @@ -378,7 +381,7 @@ impl<I: Iterator<Item = u32>> super::Frontend<I> {
let coord_handle =
self.get_expr_handle(coordinate_id, coord_lexp, ctx, emitter, block, body_idx);
let coord_type_handle = self.lookup_type.lookup(coord_lexp.type_id)?.handle;
let (coordinate, array_index, is_depth) = match ctx.type_arena[image_ty].inner {
let (coordinate, array_index, is_depth) = match ctx.module.types[image_ty].inner {
crate::TypeInner::Image {
dim,
arrayed,
Expand Down Expand Up @@ -414,7 +417,7 @@ impl<I: Iterator<Item = u32>> super::Frontend<I> {
let handle = if is_depth {
let result_ty = self.lookup_type.lookup(result_type_id)?;
// The return type of `OpImageRead` can be a scalar or vector.
match ctx.type_arena[result_ty.handle].inner {
match ctx.module.types[result_ty.handle].inner {
crate::TypeInner::Vector { size, .. } => {
let splat_expr = crate::Expression::Splat {
size,
Expand Down Expand Up @@ -493,7 +496,7 @@ impl<I: Iterator<Item = u32>> super::Frontend<I> {
let image_lexp = self.lookup_sampled_image.lookup(sampled_image_id)?;
let image_ty = ctx.get_image_expr_ty(image_lexp.image)?;
matches!(
ctx.type_arena[image_ty].inner,
ctx.module.types[image_ty].inner,
crate::TypeInner::Image {
class: crate::ImageClass::Depth { .. },
..
Expand Down Expand Up @@ -565,6 +568,7 @@ impl<I: Iterator<Item = u32>> super::Frontend<I> {
.inner
.to_expr();
let offset_handle = ctx
.module
.global_expressions
.append(offset_expr, Default::default());
offset = Some(offset_handle);
Expand Down Expand Up @@ -599,7 +603,7 @@ impl<I: Iterator<Item = u32>> super::Frontend<I> {
*flags |= sampling_bit;
}

ctx.global_arena[handle].ty
ctx.module.global_variables[handle].ty
}

crate::Expression::FunctionArgument(i) => {
Expand All @@ -613,7 +617,7 @@ impl<I: Iterator<Item = u32>> super::Frontend<I> {
*flags |= sampling_bit;
}

match ctx.type_arena[ctx.global_arena[handle].ty].inner {
match ctx.module.types[ctx.module.global_variables[handle].ty].inner {
crate::TypeInner::BindingArray { base, .. } => base,
_ => return Err(Error::InvalidGlobalVar(ctx.expressions[base].clone())),
}
Expand Down Expand Up @@ -645,60 +649,60 @@ impl<I: Iterator<Item = u32>> super::Frontend<I> {
ref other => return Err(Error::InvalidGlobalVar(other.clone())),
}

let ((coordinate, array_index), depth_ref, is_depth) = match ctx.type_arena[image_ty].inner
{
crate::TypeInner::Image {
dim,
arrayed,
class,
} => (
extract_image_coordinates(
let ((coordinate, array_index), depth_ref, is_depth) =
match ctx.module.types[image_ty].inner {
crate::TypeInner::Image {
dim,
if options.project {
ExtraCoordinate::Projection
} else if arrayed {
ExtraCoordinate::ArrayLayer
} else {
ExtraCoordinate::Garbage
arrayed,
class,
} => (
extract_image_coordinates(
dim,
if options.project {
ExtraCoordinate::Projection
} else if arrayed {
ExtraCoordinate::ArrayLayer
} else {
ExtraCoordinate::Garbage
},
coord_handle,
coord_type_handle,
ctx,
),
{
match dref_id {
Some(id) => {
let expr_lexp = self.lookup_expression.lookup(id)?;
let mut expr = self
.get_expr_handle(id, expr_lexp, ctx, emitter, block, body_idx);

if options.project {
let required_size = dim.required_coordinate_size();
let right = ctx.expressions.append(
crate::Expression::AccessIndex {
base: coord_handle,
index: required_size.map_or(1, |size| size as u32),
},
crate::Span::default(),
);
expr = ctx.expressions.append(
crate::Expression::Binary {
op: crate::BinaryOperator::Divide,
left: expr,
right,
},
crate::Span::default(),
)
};
Some(expr)
}
None => None,
}
},
coord_handle,
coord_type_handle,
ctx,
class.is_depth(),
),
{
match dref_id {
Some(id) => {
let expr_lexp = self.lookup_expression.lookup(id)?;
let mut expr =
self.get_expr_handle(id, expr_lexp, ctx, emitter, block, body_idx);

if options.project {
let required_size = dim.required_coordinate_size();
let right = ctx.expressions.append(
crate::Expression::AccessIndex {
base: coord_handle,
index: required_size.map_or(1, |size| size as u32),
},
crate::Span::default(),
);
expr = ctx.expressions.append(
crate::Expression::Binary {
op: crate::BinaryOperator::Divide,
left: expr,
right,
},
crate::Span::default(),
)
};
Some(expr)
}
None => None,
}
},
class.is_depth(),
),
_ => return Err(Error::InvalidImage(image_ty)),
};
_ => return Err(Error::InvalidImage(image_ty)),
};

let expr = crate::Expression::ImageSample {
image: si_lexp.image,
Expand Down Expand Up @@ -764,7 +768,7 @@ impl<I: Iterator<Item = u32>> super::Frontend<I> {
};

let result_type_handle = self.lookup_type.lookup(result_type_id)?.handle;
let maybe_scalar_kind = ctx.type_arena[result_type_handle].inner.scalar_kind();
let maybe_scalar_kind = ctx.module.types[result_type_handle].inner.scalar_kind();

let expr = if maybe_scalar_kind == Some(crate::ScalarKind::Sint) {
crate::Expression::As {
Expand Down Expand Up @@ -809,7 +813,7 @@ impl<I: Iterator<Item = u32>> super::Frontend<I> {
};

let result_type_handle = self.lookup_type.lookup(result_type_id)?.handle;
let maybe_scalar_kind = ctx.type_arena[result_type_handle].inner.scalar_kind();
let maybe_scalar_kind = ctx.module.types[result_type_handle].inner.scalar_kind();

let expr = if maybe_scalar_kind == Some(crate::ScalarKind::Sint) {
crate::Expression::As {
Expand Down
Loading

0 comments on commit ba1f042

Please sign in to comment.