Skip to content

Commit

Permalink
cleanup
Browse files Browse the repository at this point in the history
  • Loading branch information
molikto committed May 17, 2022
1 parent 815161d commit d31bb5f
Show file tree
Hide file tree
Showing 4 changed files with 40 additions and 29 deletions.
24 changes: 13 additions & 11 deletions crates/rustc_codegen_spirv/src/linker/inline.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,16 +10,16 @@ use super::{get_name, get_names};
use rspirv::dr::{Block, Function, Instruction, Module, ModuleHeader, Operand};
use rspirv::spirv::{FunctionControl, Op, StorageClass, Word};
use rustc_data_structures::fx::{FxHashMap, FxHashSet};
use rustc_errors::ErrorGuaranteed;
use rustc_session::Session;
use std::mem::take;

type FunctionMap = FxHashMap<Word, Function>;

pub fn inline(sess: &Session, module: &mut Module) -> super::Result<()> {
// This algorithm gets real sad if there's recursion - but, good news, SPIR-V bans recursion
if module_has_recursion(sess, module) {
return Err(rustc_errors::ErrorReported);
}
deny_recursion_in_module(sess, module)?;

let functions = module
.functions
.iter()
Expand Down Expand Up @@ -52,7 +52,7 @@ pub fn inline(sess: &Session, module: &mut Module) -> super::Result<()> {
let names = get_names(module);
for f in inlined_dont_inlines {
sess.warn(&format!(
"Function `{}` has `dont_inline` attribute, but need to be inlined because it has illegal argument or return types",
"function `{}` has `dont_inline` attribute, but need to be inlined because it has illegal argument or return types",
get_name(&names, f)
));
}
Expand Down Expand Up @@ -81,7 +81,7 @@ pub fn inline(sess: &Session, module: &mut Module) -> super::Result<()> {
}

// https://stackoverflow.com/a/53995651
fn module_has_recursion(sess: &Session, module: &Module) -> bool {
fn deny_recursion_in_module(sess: &Session, module: &Module) -> super::Result<()> {
let func_to_index: FxHashMap<Word, usize> = module
.functions
.iter()
Expand All @@ -90,7 +90,7 @@ fn module_has_recursion(sess: &Session, module: &Module) -> bool {
.collect();
let mut discovered = vec![false; module.functions.len()];
let mut finished = vec![false; module.functions.len()];
let mut has_recursion = false;
let mut has_recursion = None;
for index in 0..module.functions.len() {
if !discovered[index] && !finished[index] {
visit(
Expand All @@ -111,7 +111,7 @@ fn module_has_recursion(sess: &Session, module: &Module) -> bool {
current: usize,
discovered: &mut Vec<bool>,
finished: &mut Vec<bool>,
has_recursion: &mut bool,
has_recursion: &mut Option<ErrorGuaranteed>,
func_to_index: &FxHashMap<Word, usize>,
) {
discovered[current] = true;
Expand All @@ -121,11 +121,10 @@ fn module_has_recursion(sess: &Session, module: &Module) -> bool {
let names = get_names(module);
let current_name = get_name(&names, module.functions[current].def_id().unwrap());
let next_name = get_name(&names, module.functions[next].def_id().unwrap());
sess.err(&format!(
*has_recursion = Some(sess.err(&format!(
"module has recursion, which is not allowed: `{}` calls `{}`",
current_name, next_name
));
*has_recursion = true;
)));
break;
}

Expand Down Expand Up @@ -159,7 +158,10 @@ fn module_has_recursion(sess: &Session, module: &Module) -> bool {
})
}

has_recursion
match has_recursion {
Some(err) => Err(err),
None => Ok(()),
}
}

fn compute_disallowed_argument_and_return_types(
Expand Down
37 changes: 24 additions & 13 deletions crates/rustc_codegen_spirv/src/linker/inline_globals.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use rspirv::dr::{Instruction, Module, Operand};
use rspirv::spirv::{Op};
use rspirv::spirv::Op;
use rustc_data_structures::fx::{FxHashMap, FxHashSet};
use rustc_session::Session;

Expand Down Expand Up @@ -47,17 +47,17 @@ impl NormalizedInstructions {
bound: &mut u32,
new_root: u32,
) {
for op in &mut inst.operands {
match op {
Operand::IdRef(id) => match id_map.get(id) {
Some(new_id) => {
*id = *new_id;
}
_ => {}
},
for op in &mut inst.operands {
match op {
Operand::IdRef(id) => match id_map.get(id) {
Some(new_id) => {
*id = *new_id;
}
_ => {}
}
},
_ => {}
}
}
if let Some(id) = &mut inst.result_id {
if *id != root {
id_map.insert(*id, *bound);
Expand Down Expand Up @@ -144,7 +144,13 @@ fn inline_global_varaibles_rec(module: &mut Module) -> super::Result<bool> {
match &inst.operands[i] {
&Operand::IdRef(w) => match &function_args.get(&key) {
None => {
match get_const_arg_insts(bound, &variables, &insts, &ref_stores, w) {
match get_const_arg_insts(
bound,
&variables,
&insts,
&ref_stores,
w,
) {
Some(insts) => {
is_invalid = false;
function_args.insert(key, FunctionArg::Insts(insts));
Expand All @@ -153,8 +159,13 @@ fn inline_global_varaibles_rec(module: &mut Module) -> super::Result<bool> {
}
}
Some(FunctionArg::Insts(w2)) => {
let new_insts =
get_const_arg_insts(bound, &variables, &insts, &ref_stores, w);
let new_insts = get_const_arg_insts(
bound,
&variables,
&insts,
&ref_stores,
w,
);
match new_insts {
Some(new_insts) => {
is_invalid = new_insts != *w2;
Expand Down
4 changes: 1 addition & 3 deletions crates/rustc_codegen_spirv/src/linker/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@ mod destructure_composites;
mod duplicates;
mod entry_interface;
mod import_export_link;
mod inline_globals;
mod inline;
mod inline_globals;
mod ipo;
mod mem2reg;
mod param_weakening;
Expand Down Expand Up @@ -153,7 +153,6 @@ pub fn link(sess: &Session, mut inputs: Vec<Module>, opts: &Options) -> Result<L
std::fs::write(path, spirv_tools::binary::from_binary(&output.assemble())).unwrap();
}


// remove duplicates (https://github.com/KhronosGroup/SPIRV-Tools/blob/e7866de4b1dc2a7e8672867caeb0bdca49f458d3/source/opt/remove_duplicates_pass.cpp)
{
let _timer = sess.timer("link_remove_duplicates");
Expand Down Expand Up @@ -355,6 +354,5 @@ pub fn link(sess: &Session, mut inputs: Vec<Module>, opts: &Options) -> Result<L
};
}


Ok(output)
}
4 changes: 2 additions & 2 deletions tests/ui/lang/consts/nested-ref.stderr
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
warning: Function `nested_ref::deep_load` has `dont_inline` attribute, but need to be inlined because it has illegal argument or return types
warning: function `nested_ref::deep_load` has `dont_inline` attribute, but need to be inlined because it has illegal argument or return types

warning: Function `nested_ref::deep_transpose` has `dont_inline` attribute, but need to be inlined because it has illegal argument or return types
warning: function `nested_ref::deep_transpose` has `dont_inline` attribute, but need to be inlined because it has illegal argument or return types

warning: 2 warnings emitted

0 comments on commit d31bb5f

Please sign in to comment.