Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[naga] Remove uses of Handle::index for identifier generation from backends #5845

Merged
merged 4 commits into from
Jun 21, 2024
Merged

[naga] Remove uses of Handle::index for identifier generation from backends #5845

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
d6827b0
[naga] Introduce `Baked` newtype for writing baked expression names.
jimblandy Jun 19, 2024
462f09c
[naga hlsl-out] Use `Baked` for baked expressions.
jimblandy Jun 19, 2024
bed62ae
[naga dot-out] Use `Handle::write_prefixed` instead of `index`.
jimblandy Jun 20, 2024
2dc2f9e
[naga msl-out] Use `Handle::write_prefixed` instead of `index`.
jimblandy Jun 20, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions naga/src/arena.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,16 @@ impl<T> Handle<T> {
const unsafe fn from_usize_unchecked(index: usize) -> Self {
Handle::new(Index::new_unchecked((index + 1) as u32))
}

/// Write this handle's index to `formatter`, preceded by `prefix`.
pub fn write_prefixed(
&self,
formatter: &mut std::fmt::Formatter,
prefix: &'static str,
) -> std::fmt::Result {
formatter.write_str(prefix)?;
<usize as std::fmt::Display>::fmt(&self.index(), formatter)
}
}

/// A strongly typed range of handles.
Expand Down
75 changes: 51 additions & 24 deletions naga/src/back/dot/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -392,6 +392,32 @@ const COLORS: &[&str] = &[
"#d9d9d9",
];

struct Prefixed<T>(Handle<T>);

impl std::fmt::Display for Prefixed<crate::Expression> {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
self.0.write_prefixed(f, "e")
}
}

impl std::fmt::Display for Prefixed<crate::LocalVariable> {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
self.0.write_prefixed(f, "l")
}
}

impl std::fmt::Display for Prefixed<crate::GlobalVariable> {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
self.0.write_prefixed(f, "g")
}
}

impl std::fmt::Display for Prefixed<crate::Function> {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
self.0.write_prefixed(f, "f")
}
}

fn write_fun(
output: &mut String,
prefix: String,
Expand All @@ -405,9 +431,9 @@ fn write_fun(
for (handle, var) in fun.local_variables.iter() {
writeln!(
output,
"\t\t{}_l{} [ shape=hexagon label=\"{:?} '{}'\" ]",
"\t\t{}_{} [ shape=hexagon label=\"{:?} '{}'\" ]",
prefix,
handle.index(),
Prefixed(handle),
handle,
name(&var.name),
)?;
Expand Down Expand Up @@ -442,9 +468,9 @@ fn write_fun(
for (to, expr, label) in sg.dependencies {
writeln!(
output,
"\t\t{}_e{} -> {}_s{} [ label=\"{}\" ]",
"\t\t{}_{} -> {}_s{} [ label=\"{}\" ]",
prefix,
expr.index(),
Prefixed(expr),
prefix,
to,
label,
Expand All @@ -453,22 +479,23 @@ fn write_fun(
for (from, to) in sg.emits {
writeln!(
output,
"\t\t{}_s{} -> {}_e{} [ style=dotted ]",
"\t\t{}_s{} -> {}_{} [ style=dotted ]",
prefix,
from,
prefix,
to.index(),
Prefixed(to),
)?;
}
}

assert!(sg.calls.is_empty());
for (from, function) in sg.calls {
writeln!(
output,
"\t\t{}_s{} -> f{}_s0",
"\t\t{}_s{} -> {}_s0",
prefix,
from,
function.index(),
Prefixed(function),
)?;
}

Expand Down Expand Up @@ -688,9 +715,9 @@ fn write_function_expressions(
};
writeln!(
output,
"\t\t{}_e{} [ {}=\"{}\" label=\"{:?} {}\" ]",
"\t\t{}_{} [ {}=\"{}\" label=\"{:?} {}\" ]",
prefix,
handle.index(),
Prefixed(handle),
color_attr,
COLORS[color_id],
handle,
Expand All @@ -700,39 +727,39 @@ fn write_function_expressions(
for (key, edge) in edges.drain() {
writeln!(
output,
"\t\t{}_e{} -> {}_e{} [ label=\"{}\" ]",
"\t\t{}_{} -> {}_{} [ label=\"{}\" ]",
prefix,
edge.index(),
Prefixed(edge),
prefix,
handle.index(),
Prefixed(handle),
key,
)?;
}
match payload.take() {
Some(Payload::Arguments(list)) => {
write!(output, "\t\t{{")?;
for &comp in list {
write!(output, " {}_e{}", prefix, comp.index())?;
write!(output, " {}_{}", prefix, Prefixed(comp))?;
}
writeln!(output, " }} -> {}_e{}", prefix, handle.index())?;
writeln!(output, " }} -> {}_{}", prefix, Prefixed(handle))?;
}
Some(Payload::Local(h)) => {
writeln!(
output,
"\t\t{}_l{} -> {}_e{}",
"\t\t{}_{} -> {}_{}",
prefix,
h.index(),
Prefixed(h),
prefix,
handle.index(),
Prefixed(handle),
)?;
}
Some(Payload::Global(h)) => {
writeln!(
output,
"\t\tg{} -> {}_e{} [fillcolor=gray]",
h.index(),
"\t\t{} -> {}_{} [fillcolor=gray]",
Prefixed(h),
prefix,
handle.index(),
Prefixed(handle),
)?;
}
None => {}
Expand All @@ -759,8 +786,8 @@ pub fn write(
for (handle, var) in module.global_variables.iter() {
writeln!(
output,
"\t\tg{} [ shape=hexagon label=\"{:?} {:?}/'{}'\" ]",
handle.index(),
"\t\t{} [ shape=hexagon label=\"{:?} {:?}/'{}'\" ]",
Prefixed(handle),
handle,
var.space,
name(&var.name),
Expand All @@ -770,7 +797,7 @@ pub fn write(
}

for (handle, fun) in module.functions.iter() {
let prefix = format!("f{}", handle.index());
let prefix = Prefixed(handle).to_string();
writeln!(output, "\tsubgraph cluster_{prefix} {{")?;
writeln!(
output,
Expand Down
37 changes: 12 additions & 25 deletions naga/src/back/glsl/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ to output a [`Module`](crate::Module) into glsl
pub use features::Features;

use crate::{
back,
back::{self, Baked},
proc::{self, NameKey},
valid, Handle, ShaderStage, TypeInner,
};
Expand Down Expand Up @@ -1982,7 +1982,7 @@ impl<'a, W: Write> Writer<'a, W> {
// Also, we use sanitized names! It defense backend from generating variable with name from reserved keywords.
Some(self.namer.call(name))
} else if self.need_bake_expressions.contains(&handle) {
Some(format!("{}{}", back::BAKE_PREFIX, handle.index()))
Some(Baked(handle).to_string())
} else {
None
};
Expand Down Expand Up @@ -2310,7 +2310,7 @@ impl<'a, W: Write> Writer<'a, W> {
// This is done in `Emit` by never emitting a variable name for pointer variables
self.write_barrier(crate::Barrier::WORK_GROUP, level)?;

let result_name = format!("{}{}", back::BAKE_PREFIX, result.index());
let result_name = Baked(result).to_string();
write!(self.out, "{level}")?;
// Expressions cannot have side effects, so just writing the expression here is fine.
self.write_named_expr(pointer, result_name, result, ctx)?;
Expand All @@ -2335,7 +2335,7 @@ impl<'a, W: Write> Writer<'a, W> {
} => {
write!(self.out, "{level}")?;
if let Some(expr) = result {
let name = format!("{}{}", back::BAKE_PREFIX, expr.index());
let name = Baked(expr).to_string();
let result = self.module.functions[function].result.as_ref().unwrap();
self.write_type(result.ty)?;
write!(self.out, " {name}")?;
Expand Down Expand Up @@ -2369,7 +2369,7 @@ impl<'a, W: Write> Writer<'a, W> {
} => {
write!(self.out, "{level}")?;
if let Some(result) = result {
let res_name = format!("{}{}", back::BAKE_PREFIX, result.index());
let res_name = Baked(result).to_string();
let res_ty = ctx.resolve_type(result, &self.module.types);
self.write_value_type(res_ty)?;
write!(self.out, " {res_name} = ")?;
Expand Down Expand Up @@ -2399,7 +2399,7 @@ impl<'a, W: Write> Writer<'a, W> {
Statement::RayQuery { .. } => unreachable!(),
Statement::SubgroupBallot { result, predicate } => {
write!(self.out, "{level}")?;
let res_name = format!("{}{}", back::BAKE_PREFIX, result.index());
let res_name = Baked(result).to_string();
let res_ty = ctx.info[result].ty.inner_with(&self.module.types);
self.write_value_type(res_ty)?;
write!(self.out, " {res_name} = ")?;
Expand All @@ -2419,7 +2419,7 @@ impl<'a, W: Write> Writer<'a, W> {
result,
} => {
write!(self.out, "{level}")?;
let res_name = format!("{}{}", back::BAKE_PREFIX, result.index());
let res_name = Baked(result).to_string();
let res_ty = ctx.info[result].ty.inner_with(&self.module.types);
self.write_value_type(res_ty)?;
write!(self.out, " {res_name} = ")?;
Expand Down Expand Up @@ -2476,7 +2476,7 @@ impl<'a, W: Write> Writer<'a, W> {
result,
} => {
write!(self.out, "{level}")?;
let res_name = format!("{}{}", back::BAKE_PREFIX, result.index());
let res_name = Baked(result).to_string();
let res_ty = ctx.info[result].ty.inner_with(&self.module.types);
self.write_value_type(res_ty)?;
write!(self.out, " {res_name} = ")?;
Expand Down Expand Up @@ -3865,9 +3865,8 @@ impl<'a, W: Write> Writer<'a, W> {
// Define our local and start a call to `clamp`
write!(
self.out,
"int {}{}{} = clamp(",
back::BAKE_PREFIX,
expr.index(),
"int {}{} = clamp(",
Baked(expr),
CLAMPED_LOD_SUFFIX
)?;
// Write the lod that will be clamped
Expand Down Expand Up @@ -4205,13 +4204,7 @@ impl<'a, W: Write> Writer<'a, W> {
// `textureSize` call, but this needs to be the clamped lod, this should
// have been generated earlier and put in a local.
if class.is_mipmapped() {
write!(
self.out,
", {}{}{}",
back::BAKE_PREFIX,
handle.index(),
CLAMPED_LOD_SUFFIX
)?;
write!(self.out, ", {}{}", Baked(handle), CLAMPED_LOD_SUFFIX)?;
}
// Close the `textureSize` call
write!(self.out, ")")?;
Expand All @@ -4229,13 +4222,7 @@ impl<'a, W: Write> Writer<'a, W> {
// Add the clamped lod (if present) as the second argument to the
// image load function.
if level.is_some() {
write!(
self.out,
", {}{}{}",
back::BAKE_PREFIX,
handle.index(),
CLAMPED_LOD_SUFFIX
)?;
write!(self.out, ", {}{}", Baked(handle), CLAMPED_LOD_SUFFIX)?;
}

// If a sample argument is needed we need to clamp it between 0 and
Expand Down
16 changes: 8 additions & 8 deletions naga/src/back/hlsl/writer.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use super::{
BackendResult, Error, Options,
};
use crate::{
back,
back::{self, Baked},
proc::{self, NameKey},
valid, Handle, Module, ScalarKind, ShaderStage, TypeInner,
};
Expand Down Expand Up @@ -1410,7 +1410,7 @@ impl<'a, W: fmt::Write> super::Writer<'a, W> {
// Also, we use sanitized names! It defense backend from generating variable with name from reserved keywords.
Some(self.namer.call(name))
} else if self.need_bake_expressions.contains(&handle) {
Some(format!("_expr{}", handle.index()))
Some(Baked(handle).to_string())
} else {
None
};
Expand Down Expand Up @@ -1891,7 +1891,7 @@ impl<'a, W: fmt::Write> super::Writer<'a, W> {
write!(self.out, "{level}")?;
if let Some(expr) = result {
write!(self.out, "const ")?;
let name = format!("{}{}", back::BAKE_PREFIX, expr.index());
let name = Baked(expr).to_string();
let expr_ty = &func_ctx.info[expr].ty;
match *expr_ty {
proc::TypeResolution::Handle(handle) => self.write_type(module, handle)?,
Expand Down Expand Up @@ -1922,7 +1922,7 @@ impl<'a, W: fmt::Write> super::Writer<'a, W> {
let res_name = match result {
None => None,
Some(result) => {
let name = format!("{}{}", back::BAKE_PREFIX, result.index());
let name = Baked(result).to_string();
match func_ctx.info[result].ty {
proc::TypeResolution::Handle(handle) => {
self.write_type(module, handle)?
Expand Down Expand Up @@ -1992,7 +1992,7 @@ impl<'a, W: fmt::Write> super::Writer<'a, W> {
Statement::WorkGroupUniformLoad { pointer, result } => {
self.write_barrier(crate::Barrier::WORK_GROUP, level)?;
write!(self.out, "{level}")?;
let name = format!("_expr{}", result.index());
let name = Baked(result).to_string();
self.write_named_expr(module, pointer, name, result, func_ctx)?;

self.write_barrier(crate::Barrier::WORK_GROUP, level)?;
Expand Down Expand Up @@ -2099,7 +2099,7 @@ impl<'a, W: fmt::Write> super::Writer<'a, W> {
Statement::RayQuery { .. } => unreachable!(),
Statement::SubgroupBallot { result, predicate } => {
write!(self.out, "{level}")?;
let name = format!("{}{}", back::BAKE_PREFIX, result.index());
let name = Baked(result).to_string();
write!(self.out, "const uint4 {name} = ")?;
self.named_expressions.insert(result, name);

Expand All @@ -2118,7 +2118,7 @@ impl<'a, W: fmt::Write> super::Writer<'a, W> {
} => {
write!(self.out, "{level}")?;
write!(self.out, "const ")?;
let name = format!("{}{}", back::BAKE_PREFIX, result.index());
let name = Baked(result).to_string();
match func_ctx.info[result].ty {
proc::TypeResolution::Handle(handle) => self.write_type(module, handle)?,
proc::TypeResolution::Value(ref value) => {
Expand Down Expand Up @@ -2182,7 +2182,7 @@ impl<'a, W: fmt::Write> super::Writer<'a, W> {
} => {
write!(self.out, "{level}")?;
write!(self.out, "const ")?;
let name = format!("{}{}", back::BAKE_PREFIX, result.index());
let name = Baked(result).to_string();
match func_ctx.info[result].ty {
proc::TypeResolution::Handle(handle) => self.write_type(module, handle)?,
proc::TypeResolution::Value(ref value) => {
Expand Down
18 changes: 16 additions & 2 deletions naga/src/back/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,12 +28,26 @@ pub mod pipeline_constants;
pub const COMPONENTS: &[char] = &['x', 'y', 'z', 'w'];
/// Indent for backends.
pub const INDENT: &str = " ";
/// Prefix used for baking.
pub const BAKE_PREFIX: &str = "_e";

/// Expressions that need baking.
pub type NeedBakeExpressions = crate::FastHashSet<crate::Handle<crate::Expression>>;

/// A type for displaying expression handles as baking identifiers.
///
/// Given an [`Expression`] [`Handle`] `h`, `Baked(h)` implements
/// [`std::fmt::Display`], showing the handle's index prefixed by
/// [`BAKE_PREFIX`].
///
/// [`Expression`]: crate::Expression
/// [`Handle`]: crate::Handle
struct Baked(crate::Handle<crate::Expression>);

impl std::fmt::Display for Baked {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
self.0.write_prefixed(f, "_e")
}
}

/// Specifies the values of pipeline-overridable constants in the shader module.
///
/// If an `@id` attribute was specified on the declaration,
Expand Down
Loading
Loading